On 4/11/22 10:57 AM, Cole Robinson wrote:
On 3/23/22 3:36 PM, Tyler Fanelli wrote:
This an RFC discussing a new API, virDomainGetSevAttestationReport (along with a
virsh command "domgetsevreport"), with initial QEMU support via the
"query-sev-attestation-report" QAPI mechanism. "query-sev-attestation-report" is
supplied a base64-encoded 16 byte "mnonce" string as input, with a purpose of
being embedded into the attestation report to provide protection.
My main point of concern is the design/communication of the virTypedParameterPtr
exchanged between the client and libvirtd and how they interact together, as I
have seen no other API follow the method I used. Namely, the same
virTypedParameterPtr is used for both input _AND_ output. The same
virTypedParameterPtr containing the original mnonce string inputted to the API
is
also used to contain the attestation report upon being returned from the API.
This contrasts with much of the APIs I've noticed, which use a
virTypedParameterPtr for either input or output, but not both.
This patch is not final, as I still would like some human-readable outputting
and storage of the attestation report.
Looking for thoughts on the design of this API, as well as suggested
improvements.
The proposed API name contains Sev, when all the existing domain APIs
have generic names: virDomainGetLaunchSecurityInfo,
virDomainSetLaunchSecurityState.
I was thinking it would be nice to avoid that Sev specific bit. So I
looked at upcoming SNP and TDX qemu patches to see if they add any qmp
commands that take input and return a lot of output. Then maybe it would
make sense to name this something like
virDomainGetLaunchSecurityInfoWithParams which we could use more in the
future.
qemu SNP patches: https://github.com/AMDESE/qemu/tree/snp-v3
- Only extend existing query-sev and query-sev-capabilities
qemu TDX patches: https://github.com/intel/qemu-tdx
- Adds query-tdx and query-tdx-capabilities, both with no input
So, that doesn't help.
What about adding the attestation report to
virDomainGetLaunchSecurityInfo? If that route is taken, there would
probably need to be some extra logic added to decipher getting launch
security info on SEV vs. SGX/TDX, right?
After that, my question is, what query-sev-attestation-report adds. The
kernel commit explains what it is:
https://github.com/torvalds/linux/commit/2c07ded06427dd3339278487a1413d5e478f05f9
The SEV FW version >= 0.23 added a new command that can be used to query
the attestation report containing the SHA-256 digest of the guest memory
encrypted through the KVM_SEV_LAUNCH_UPDATE_{DATA, VMSA} commands and
sign the report with the Platform Endorsement Key (PEK).
See the SEV FW API spec section 6.8 for more details.
Note there already exist a command (KVM_SEV_LAUNCH_MEASURE) that can be
used to get the SHA-256 digest. The main difference between the
KVM_SEV_LAUNCH_MEASURE and KVM_SEV_ATTESTATION_REPORT is that the latter
can be called while the guest is running and the measurement value is
signed with PEK.
So it is LAUNCH_MEASURE, available at VM runtime, signed with an extra key.
qemu caches the LAUNCH_MEASURE value at VM startup and lets us query it
with qmp any time, so I don't think runtime access matters.
I'm surprised by this. I was under the impression that LAUNCH_MEASURE
always had to be called when a VM is paused.
Maybe the extra key signing is a security fix or something. I haven't
figured it out.
Signing with the PEK also allows a user to verify the root of trust
between the owner and the platform.
But as is it's not clear what this buys us over the launch measurement
we already report with virDomainGetLaunchSecurityInfo
If we figure out what the point of this is, IMO we can more easily
reason about whether it makes sense to add a Sev specific libvirt API,
and whether we need virTypedParams for both input and output. For
example if the API really is specific to this one and only KVM ioctl/QMP
command, we could hardcode the parameters and skip the virTypedParams
question entirely.
Interesting, although wouldn't hardcoding an nonce basically render it
useless? User-specified nonce would allow a user to verify that their
call was propagated to firmware at that instance. If they can't supply
the nonce, they can't verify it's an attestation report from that
specific call.
CCing danpb for his thoughts
- Cole
Tyler.
--
Tyler Fanelli (tfanelli)
