docs/manpages/virsh.rst | 18 ++++ include/libvirt/libvirt-domain.h | 22 +++++ src/driver-hypervisor.h | 7 ++ src/libvirt-domain.c | 63 ++++++++++++++ src/libvirt_public.syms | 4 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 86 +++++++++++++++++++ src/qemu/qemu_monitor.c | 11 +++ src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 40 +++++++++ src/qemu/qemu_monitor_json.h | 5 ++ src/remote/remote_daemon_dispatch.c | 44 ++++++++++ src/remote/remote_driver.c | 55 ++++++++++++ src/remote/remote_protocol.x | 21 ++++- src/remote_protocol-structs | 12 +++ .../caps_6.1.0.x86_64.xml | 1 + .../caps_6.2.0.x86_64.xml | 1 + .../caps_7.0.0.x86_64.xml | 1 + tools/virsh-domain.c | 68 +++++++++++++++ 20 files changed, 466 insertions(+), 1 deletion(-)
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. Tyler Fanelli (5): libvirt: Introduce virDomainGetSevAttestationReport public API remote: add RPC support for the virDomainGetSevAttestationReport API qemu_capabilities: Introduce QEMU_CAPS_SEV_GET_ATTESTATION_REPORT qemu: Implement the virDomainGetSevAttestationReport API tools: add domgetsevreport virsh command docs/manpages/virsh.rst | 18 ++++ include/libvirt/libvirt-domain.h | 22 +++++ src/driver-hypervisor.h | 7 ++ src/libvirt-domain.c | 63 ++++++++++++++ src/libvirt_public.syms | 4 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 86 +++++++++++++++++++ src/qemu/qemu_monitor.c | 11 +++ src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 40 +++++++++ src/qemu/qemu_monitor_json.h | 5 ++ src/remote/remote_daemon_dispatch.c | 44 ++++++++++ src/remote/remote_driver.c | 55 ++++++++++++ src/remote/remote_protocol.x | 21 ++++- src/remote_protocol-structs | 12 +++ .../caps_6.1.0.x86_64.xml | 1 + .../caps_6.2.0.x86_64.xml | 1 + .../caps_7.0.0.x86_64.xml | 1 + tools/virsh-domain.c | 68 +++++++++++++++ 20 files changed, 466 insertions(+), 1 deletion(-) -- 2.34.1
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.
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.
Maybe the extra key signing is a security fix or something. I haven't
figured it out.
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.
CCing danpb for his thoughts
- Cole
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)
On Thu, Apr 14, 2022 at 02:46:38PM -0400, Tyler Fanelli wrote:
> 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.
Yep, QEMU registers a callback that fetches the launch measurement
from SEV platform when the machine init is done. The query-sev-launch-measure
command always returns cached data, so there's no restriction on when we can
call it from QEMU.
> > 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.
The guest owner needs to acquire the PDH before they launch their guest,
in order to generate the launch blob. The PDH is signed with the PEK,
so they will have surely verified the root of trust with the platform
before they even generate the launch blob for the VM. The generated
launch blob can't be used on any other host, because the other host
won't have the same PDH.
If the launch measurement is signed with the PEK, the guest owner has
the option of postponing their validation of the root of trust until
after the VM is launched. Is that something we need to be able to
deal with ? I'm not sure why one would want to postpone validation
gven that we're already forced to do other stuff else upfront with
SEV/SEV-ES. In the absence of a clearly stated requirement from an
application I think we can ignore this.
SEV-SNP is of course very different with its guest initiated post
launch attestation.
> > 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.
The launch blob contains a unique TIK/TEK pair, so if the launch
measurement validates, the guest owner knows it is associated with
a running VM that was created with their designated launch blob.
A nonce is usually needed to avoid replay attacks, but I'm not seeing
what attack vector is actually present in the SEV/SEV-ES scenario,
since AFAIK, the attestation report content never changes once the
VM is running.
Overall I'm not seeing the need for this API with SEV/SEV-ES at least,
and with SEV-SNP IIUC the attestation report is not available to the
host, only to the guest ?
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 4/20/22 5:45 AM, Daniel P. Berrangé wrote: > On Thu, Apr 14, 2022 at 02:46:38PM -0400, Tyler Fanelli wrote: >> On 4/11/22 10:57 AM, Cole Robinson wrote: >>> 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. > The guest owner needs to acquire the PDH before they launch their guest, > in order to generate the launch blob. The PDH is signed with the PEK, > so they will have surely verified the root of trust with the platform > before they even generate the launch blob for the VM. The generated > launch blob can't be used on any other host, because the other host > won't have the same PDH. > > If the launch measurement is signed with the PEK, the guest owner has > the option of postponing their validation of the root of trust until > after the VM is launched. Is that something we need to be able to > deal with ? I'm not sure why one would want to postpone validation > gven that we're already forced to do other stuff else upfront with > SEV/SEV-ES. In the absence of a clearly stated requirement from an > application I think we can ignore this. > > SEV-SNP is of course very different with its guest initiated post > launch attestation. I see, then the use-case of postponing the validation until after VM launch doesn't make much sense for SEV/SEV-ES given that everything should be verified upfront. > >>> 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. > The launch blob contains a unique TIK/TEK pair, so if the launch > measurement validates, the guest owner knows it is associated with > a running VM that was created with their designated launch blob. > > A nonce is usually needed to avoid replay attacks, but I'm not seeing > what attack vector is actually present in the SEV/SEV-ES scenario, > since AFAIK, the attestation report content never changes once the > VM is running. > > Overall I'm not seeing the need for this API with SEV/SEV-ES at least, > and with SEV-SNP IIUC the attestation report is not available to the > host, only to the guest ? Realizing that my assumption of LAUNCH_MEASURE needing to be called while VM is paused is false, I tend to agree. With that in mind, what is the point of "query-sev-attestation-report" in QEMU? What was it's original purpose if it offers no real benefits compared to "query-sev-launch-measure"? > > > > With regards, > Daniel -- Tyler Fanelli (tfanelli)
On Thu, Apr 21, 2022 at 12:35:27PM -0400, Tyler Fanelli wrote: > On 4/20/22 5:45 AM, Daniel P. Berrangé wrote: > > > > 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. > > The launch blob contains a unique TIK/TEK pair, so if the launch > > measurement validates, the guest owner knows it is associated with > > a running VM that was created with their designated launch blob. > > > > A nonce is usually needed to avoid replay attacks, but I'm not seeing > > what attack vector is actually present in the SEV/SEV-ES scenario, > > since AFAIK, the attestation report content never changes once the > > VM is running. > > > > Overall I'm not seeing the need for this API with SEV/SEV-ES at least, > > and with SEV-SNP IIUC the attestation report is not available to the > > host, only to the guest ? > > Realizing that my assumption of LAUNCH_MEASURE needing to be called while VM > is paused is false, I tend to agree. With that in mind, what is the point of > "query-sev-attestation-report" in QEMU? What was it's original purpose if it > offers no real benefits compared to "query-sev-launch-measure"? I'm thinking the author didn't rememeber that we cached LAUNCH_MEASURE in QEMU. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 4/14/22 2:46 PM, Tyler Fanelli wrote:
> 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?
>
The problem is virDomainGetLaunchSecurityInfo doesn't have any way to
pass in the nonce, so we can't reuse that API as is.
>>
>>
>> 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.
>>
And with a nonce passed in, I forgot to mention that. That's another bit
I'm not sure what it adds over the traditional measurement
>> 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 I don't understand what that means in practice. I figured key
management via the session blob already took care of this, but I haven't
tried to wrap my head around the details.
>>
>> 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.
>
Sorry if I was unclear, I didn't mean hardcoding a specific nonce value.
I meant that, if we decide we there's not much value in making this API
generic, then we can strip out the virTypedParams usage and make the
signature closer to:
int
virDomainGetSevAttestationReport(virDomainPtr domain,
const char *mnonce,
char *report);
Thanks,
Cole
Just a quick ping so this patchset doesn't get lost in the list -- may I receive a review on this? 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. > > > Tyler Fanelli (5): > libvirt: Introduce virDomainGetSevAttestationReport public API > remote: add RPC support for the virDomainGetSevAttestationReport API > qemu_capabilities: Introduce QEMU_CAPS_SEV_GET_ATTESTATION_REPORT > qemu: Implement the virDomainGetSevAttestationReport API > tools: add domgetsevreport virsh command > > docs/manpages/virsh.rst | 18 ++++ > include/libvirt/libvirt-domain.h | 22 +++++ > src/driver-hypervisor.h | 7 ++ > src/libvirt-domain.c | 63 ++++++++++++++ > src/libvirt_public.syms | 4 + > src/qemu/qemu_capabilities.c | 2 + > src/qemu/qemu_capabilities.h | 1 + > src/qemu/qemu_driver.c | 86 +++++++++++++++++++ > src/qemu/qemu_monitor.c | 11 +++ > src/qemu/qemu_monitor.h | 5 ++ > src/qemu/qemu_monitor_json.c | 40 +++++++++ > src/qemu/qemu_monitor_json.h | 5 ++ > src/remote/remote_daemon_dispatch.c | 44 ++++++++++ > src/remote/remote_driver.c | 55 ++++++++++++ > src/remote/remote_protocol.x | 21 ++++- > src/remote_protocol-structs | 12 +++ > .../caps_6.1.0.x86_64.xml | 1 + > .../caps_6.2.0.x86_64.xml | 1 + > .../caps_7.0.0.x86_64.xml | 1 + > tools/virsh-domain.c | 68 +++++++++++++++ > 20 files changed, 466 insertions(+), 1 deletion(-) > -- Tyler Fanelli (tfanelli)
© 2016 - 2024 Red Hat, Inc.