search

coconut-svsm / svsm

COCONUT-SVSM
MIT License
122 stars 43 forks source link

Ensure IGVM output is compatible with QEMU and calculate launch measurement #294

Closed roy-hopkins closed 8 months ago

roy-hopkins commented 8 months ago

This PR introduces a new tool igvmmeasure that parses an IGVM file and calculates the launch digest which should match the launch measurement included as part of the attestation report from the platform.

By introducing this tool, it created the opportunity to verify that the directives within the IGVM file produced by the build process are properly sent as initial guest state, exactly as described and in the correct order. It was found that there were some discrepancies between the current IGVM file and the QEMU loader/KVM launch process that resulted in a mismatch between the expected and actual digest.

The first two commits in this PR fix these issues which both relate to the VMSA then proceed to introduce the measurement tool and introduce it into the build. An option has also been added to the tool to catch these inconsistencies in future changes to the IGVM file, causing the build to abort if detected.

Note that in order to obtain a matching measurement, fixes are required on top of the existing coconut/qemu. I will be raising a new PR with these changes shortly.

roy-hopkins commented 8 months ago

@msft-jlange Please can you check if the changes I've made here affect Hyper-V: https://github.com/coconut-svsm/svsm/pull/294/commits/43ebc3b4d9370664ae14a897e31997c355d8e6d2

I can make these hypervisor specific if necessary.

roy-hopkins commented 8 months ago

@osteffenrh - I heard on the OC3 talk yesterday that you are planning to update https://github.com/virtee/sev-snp-measure to work with IGVM. You might want to take a look at this PR to see if it meets your needs.

osteffenrh commented 8 months ago

@osteffenrh - I heard on the OC3 talk yesterday that you are planning to update https://github.com/virtee/sev-snp-measure to work with IGVM. You might want to take a look at this PR to see if it meets your needs.

@roy-hopkins, Maybe that is no longer needed, now that you are contributing this PR here. :-D We pre-recoded the talk a while back...

msft-jlange commented 8 months ago

@msft-jlange Please can you check if the changes I've made here affect Hyper-V: 43ebc3b

We have test environments in which we need to be able to boot Coconut without executing any guest requests, and your changes to obtain an attestation report break that environment. Can you please add some IGVM parameter (ideally triggered from the command line) which can be used to suppress the guest request?

I'm aware of other future use cases for Coconut where boot-time guest requests will prove problematic, so I think we want to avoid establishing a pattern where guest requests are invoked unless we know that the execution environment is one that demands their use. Encoding this in the IGVM parameters would serve to capture that.

Also, I'm not entirely clear on why you want to capture the VMPL 0 measurement via attestation. If you generate an SNP_ID_BLOCK as part of the IGVM builder, then you can prevent the VM from even launching if its measurement is incorrect. At some point, it would be helpful to understand what problem you're trying to solve by performing this early attestation and we can see if there is an equivalent way to solve the problem that doesn't require capturing an attestation report.

roy-hopkins commented 8 months ago

Also, I'm not entirely clear on why you want to capture the VMPL 0 measurement via attestation. If you generate an SNP_ID_BLOCK as part of the IGVM builder, then you can prevent the VM from even launching if its measurement is incorrect. At some point, it would be helpful to understand what problem you're trying to solve by performing this early attestation and we can see if there is an equivalent way to solve the problem that doesn't require capturing an attestation report.

The generation of the attestation report at this stage is purely informational and was added after discussions around making progress towards generating SNP_ID_BLOCKs. It serves as a rehearsal for possible future functionality where the SVSM will use the attestation report to request access to keys via the host.

At this stage I don't think there is any need to complicate this by making it conditional and I'd be happy to drop this commit from the PR.

jepio commented 8 months ago

@roy-hopkins: how about adding a igvmmeasure/.cargo/config.toml:

[build]
target = "x86_64-unknown-linux-gnu"

so that cargo build can be directly executed from the igvmmeasure directory?

malt3 commented 8 months ago

There seems to be something off here. I just wrote my own tool that should perform the same job. So far, I was only able to successfully extract the ld value from the id block:

https://github.com/microsoft/igvm/blob/ac37f5e5658262f3e377f3f6019c9e631a32f644/igvm/src/lib.rs#L706

I have an IGVM file I can boot, that has a different value for the launch measurement in the id block than what igvmmeasure returns.

After launching the VM, reports I get from the VM also show the measurement value from the id block.

I think this tool is measuring something incorrectly.

This is an IGVM file I got from an Azure AKS node for testing purposes:

https://cdn.confidential.cloud/contrast/node-components/2024-03-13/kata-containers-igvm.img

ID block and report say: 4a7cb0294cbeeb2dae9cad42ba97e91133f929da970aa211119389f59bc9e2560f936ecce92cbd55f03ab3c019afd591

igvmmeasure reports:

$ igvmmeasure -v kata-containers-igvm.img
gpa 0xa0000 len 0x40000 (Zero page)
gpa 0xe0000 len 0x1000 (Normal page)
gpa 0xe1000 len 0x1f000 (Zero page)
gpa 0x100000 len 0x9000 (Normal page)
gpa 0x800000 len 0x1000 (Cpuid page)
gpa 0x801000 len 0x1000 (Secrets page)
gpa 0x802000 len 0x1000 (Unmeasured page)
gpa 0x803000 len 0x1000 (VMSA page)
gpa 0x1000000 len 0x713000 (Normal page)
gpa 0x1723000 len 0x4000 (Zero page)
gpa 0x367b000 len 0x7000 (Zero page)
gpa 0x3682000 len 0x4000 (Normal page)
gpa 0x3686000 len 0x1000 (Zero page)
gpa 0x3687000 len 0x1000 (Normal page)

===============================================================================================================
igvmmeasure 'kata-containers-igvm.img'
Launch Digest: 871990F28526B47CAF4443D7AA06B0D3B145D7B266A9E00B49FFAE755763123E23FBD70AA8317011FC550F6F12FB7ACC
===============================================================================================================
roy-hopkins commented 8 months ago

There seems to be something off here. ...

This is really useful information, thank you. I'll take a look at that IGVM file and see if I can work out why igvmmeasure results in a different value. The tool is designed to work with IGVM files from our own igvmbuilder so I wonder if there are some directives that are either not being handled correctly, or not handled at all.

I'll update here with what I find.

malt3 commented 8 months ago

Would it be a good idea to print a warning (or even return with a non-zero exit code) if the ld value does not match with the result of measuring the actual pages? It feels to me like this might catch other issues at some point.

roy-hopkins commented 8 months ago

By chance at the same time as @jepio's observation about zero pages above, I managed to get the output of igvmmeasure to match the ID block LD value by experimenting with measuring a page of zeros instead of using PAGE_TYPE_ZERO.

As far as I am concerned, the QEMU behaviour is correct and we shouldn't be populating a page of zeros into guest memory when we encounter empty page data in the IGVM file, but should instead use the SNP page type of PAGE_TYPE_ZERO. However, in order to support the virtualisation stack you are using I can add an option to the measure tool to change the behaviour when measuring these pages.

roy-hopkins commented 8 months ago

Would it be a good idea to print a warning (or even return with a non-zero exit code) if the ld value does not match with the result of measuring the actual pages? It feels to me like this might catch other issues at some point.

Yes, that does make sense. The COCONUT IGVM builder does not generate an ID block yet but it will at some point, so I'll add a check that it matches if it is present.

malt3 commented 8 months ago

However, in order to support the virtualisation stack you are using I can add an option to the measure tool to change the behaviour when measuring these pages.

I would appreciate that! I personally can’t comment on ABI compliance (just a user of the platform). @jepio is probably in a better position to comment on that.

The virtualization stack is cloud-hypervisor with the hyperv backend on Azure with nested virt (the Azure AKS CoCo offering)

roy-hopkins commented 8 months ago

Changes in the latest push:

This hopefully resolves all the current comments on the PR.

I see a rebase on main is required so I'll push this shortly [done].