[PATCH] qemu: Report sev measurement value and nonce explicitly
The value returned by qemu's query-sev-launch-measure comes
straight from the LAUNCH_MEASURE SEV firmware command. It's two
values packed together: first 32 bytes is the launch measurement,
last 16 bytes is the nonce.
This combined value is really just an artifact of the return value
of the firmware command, it has no direct usage. Users want the two
individual values. But because qemu and libvirt do not separate them
apart, every app that wants to process this value will have to do
it manually.
This performs the split for the user, and delivers the values in two
new TYPED_PARAM fields: sev-measurement-value, sev-measurement-nonce
Signed-off-by: Cole Robinson
---
include/libvirt/libvirt-domain.h | 22 ++
src/qemu/qemu_driver.c | 23 +++
2 files changed, 45 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 8357aea797..55723ba150 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -6317,6 +6317,28 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
*/
# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_SET_ADDRESS
"sev-secret-set-address"
+/**
+ * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_VALUE:
+ *
+ * Macro represents the measurement value of the SEV guest,
+ * extracted from the compound VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT
value,
+ * as VIR_TYPED_PARAM_STRING.
+ *
+ * Since: 8.9.0
+ */
+# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_VALUE
"sev-measurement-value"
+
+/**
+ * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_NONCE:
+ *
+ * Macro represents the measurement nonce of the SEV guest,
+ * extracted from the compound VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT
value,
+ * as VIR_TYPED_PARAM_STRING.
+ *
+ * Since: 8.9.0
+ */
+# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_NONCE
"sev-measurement-nonce"
+
int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
virTypedParameterPtr *params,
int *nparams,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 40d23b5723..590e8f3fab 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19951,10 +19951,14 @@ qemuDomainGetSEVInfo(virDomainObj *vm,
int ret = -1;
int rv;
g_autofree char *tmp = NULL;
+g_autofree char *measurement = NULL;
+g_autofree char *measurement_val = NULL;
+g_autofree char *nonce = NULL;
unsigned int apiMajor = 0;
unsigned int apiMinor = 0;
unsigned int buildID = 0;
unsigned int policy = 0;
+size_t measurement_size = 0;
int maxpar = 0;
virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
@@ -19982,6 +19986,17 @@ qemuDomainGetSEVInfo(virDomainObj *vm,
if (rv < 0)
goto endjob;
+measurement = (char *) g_base64_decode(tmp, _size);
+if (measurement_size != 48) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected SEV measurement size %zu, expected 48"),
+ measurement_size);
+goto endjob;
+}
+
+measurement_val = g_base64_encode((unsigned char *) measurement, 32);
+nonce = g_base64_encode((unsigned char *) measurement + 32, 16);
+
if (virTypedParamsAddString(params, nparams, ,
VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT,
tmp) < 0)
@@ -20002,6 +20017,14 @@ qemuDomainGetSEVInfo(virDomainObj *vm,
VIR_DOMAIN_LAUNCH_SECURITY_SEV_POLICY,
policy) < 0)
goto endjob;
+if (virTypedParamsAddString(params, nparams, ,
+
VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_VALUE,
+measurement_val) < 0)
+goto endjob;
+if (virTypedParamsAddString(params, nparams, ,
+
VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_NONCE,
+nonce) < 0)
+goto endjob;
ret = 0;
--
2.37.3
Re: [libvirt PATCH 12/12] docs/manpages: add checklist of problems for SEV attestation
On 10/7/22 7:43 AM, Daniel P. Berrangé wrote: > Despite efforts to make the virt-qemu-sev-validate tool friendly, it is > a certainty that almost everyone who tries it will hit false negative > results, getting a failure despite the VM being trustworthy. > > Diagnosing these problems is no easy matter, especially for those not > familiar with SEV/SEV-ES in general. This extra docs text attempts to > set out a checklist of items to look at to identify what went wrong. > > Signed-off-by: Daniel P. Berrangé > --- > docs/manpages/virt-qemu-sev-validate.rst | 112 +++ > 1 file changed, 112 insertions(+) > > diff --git a/docs/manpages/virt-qemu-sev-validate.rst > b/docs/manpages/virt-qemu-sev-validate.rst > index 7542bea9aa..e0c18f2d20 100644 > --- a/docs/manpages/virt-qemu-sev-validate.rst > +++ b/docs/manpages/virt-qemu-sev-validate.rst > @@ -437,6 +437,118 @@ inject a disk password on success: > --domain fedora34x86_64 \ > --disk-password passwd.txt > > +COMMON MISTAKES CHECKLIST > += > + > +The complexity of configuring a guest and validating its boot measurement > +means it is very likely to see the failure:: > + > + ERROR: Measurement does not match, VM is not trustworthy > + > +This error message assumes the worst, but in most cases will failure will be > +a result of either mis-configuring the guest, or passing the wrong > information > +when trying to validate it. The following information is a guide for what > +items to check in order to stand the best chance of diagnosing the problem > + > +* Check the VM configuration for the DH certificate and session > + blob in the libvirt guest XML. > + > + The content for these fields should be in base64 format, which is > + what ``sevctl session`` generates. Other tools may generate the files > + in binary format, so ensure it has been correctly converted to base64. > + > +* Check the VM configuration policy value matches the session blob > + > + The value in libvirt guest XML has to match the value > + passed to the ``sevctl session`` command. > + FWIW In this case, qemu will explicitly error. From 7.0.0-6.fc36: -accel kvm: sev_launch_start: LAUNCH_START ret=1 fw_error=11 'Bad measurement' I think it's worth putting some subset of that qemu error string at the top of this section too. If users hit it, going through the checklist here may solve their issue. For example, If you're flailing around with sevctl like I have, some of the sub commands will invalidate all your previous generated session/dhCert blobs, and subsequent VM boots will fail as above. `sevctl reset` and/or `sevctl rotate`. That's probably obscure enough to not need documenting, but if the first item here leads to re-running sevctl session then you'll fix your problem :) Thanks, Cole
Re: [libvirt PATCH 08/12] tools: load CPU count and CPU SKU from libvirt
On 10/7/22 7:43 AM, Daniel P. Berrangé wrote:
> When validating a SEV-ES guest, we need to know the CPU count and VMSA
> state. We can get the CPU count directly from libvirt's guest info. The
> VMSA state can be constructed automatically if we query the CPU SKU from
> host capabilities XML. Neither of these is secure, however, so this
> behaviour is restricted.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> docs/manpages/virt-qemu-sev-validate.rst | 4
> tools/virt-qemu-sev-validate.py | 23 +++
> 2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/docs/manpages/virt-qemu-sev-validate.rst
> b/docs/manpages/virt-qemu-sev-validate.rst
> index 7ba7323e13..fcc13d68c8 100644
> --- a/docs/manpages/virt-qemu-sev-validate.rst
> +++ b/docs/manpages/virt-qemu-sev-validate.rst
> @@ -356,7 +356,6 @@ Validate the measurement of a SEV-ES SMP guest booting
> from disk:
>
> # virt-dom-sev-validate \
> --insecure \
> - --num-cpus 2 \
> --vmsa-cpu0 vmsa0.bin \
> --vmsa-cpu1 vmsa1.bin \
> --tk this-guest-tk.bin \
> @@ -369,9 +368,6 @@ automatically constructed VMSA:
>
> # virt-dom-sev-validate \
> --insecure \
> - --cpu-family 23 \
> - --cpu-model 49 \
> - --cpu-stepping 0 \
> --tk this-guest-tk.bin \
> --domain fedora34x86_64
>
> diff --git a/tools/virt-qemu-sev-validate.py b/tools/virt-qemu-sev-validate.py
> index 2505aff07f..5da1353e60 100755
> --- a/tools/virt-qemu-sev-validate.py
> +++ b/tools/virt-qemu-sev-validate.py
> @@ -869,6 +869,14 @@ class LibvirtConfidentialVM(ConfidentialVM):
> if self.policy is None:
> self.policy = sevinfo["sev-policy"]
>
> +if self.is_sev_es() and self.num_cpus is None:
> +if secure:
> +raise InsecureUsageException(
> +"Using CPU count from guest is not secure")
> +
> +info = self.dom.info()
> +self.num_cpus = info[3]
> +
> if self.firmware is None:
> if remote:
> raise UnsupportedUsageException(
> @@ -914,6 +922,21 @@ class LibvirtConfidentialVM(ConfidentialVM):
> "Using cmdline string from XML is not secure")
> self.kernel_table.load_cmdline(cmdlinenodes[0].text)
>
> +capsxml = self.conn.getCapabilities()
> +capsdoc = etree.fromstring(capsxml)
> +
> +if self.is_sev_es() and self.vmsa_cpu0 is None:
> +if secure:
> +raise InsecureUsageException(
> +"Using CPU SKU from capabilities is not secure")
> +
> +sig = capsdoc.xpath("/capabilities/host/cpu/signature")
> +if len(sig) == 1:
If this is missing, I'd make it fatal, libvirtd isn't new enough. Could
happen if talking to a remote machine (or testing the script while f36
fedora libvirtd is running, which I did :) ) . It's going to fail later
anyways.
- Cole
> +cpu_family = int(sig[0].get("family"))
> +cpu_model = int(sig[0].get("model"))
> +cpu_stepping = int(sig[0].get("stepping"))
> +self.build_vmsas(cpu_family, cpu_model, cpu_stepping)
> +
>
> def parse_command_line():
> parser = argparse.ArgumentParser(
Re: [libvirt PATCH 00/12] tools: provide virt-qemu-sev-validate for SEV(-ES) launch attestation
On 10/7/22 7:42 AM, Daniel P. Berrangé wrote: > The libvirt QEMU driver provides all the functionality required for > launching a guest on AMD SEV(-ES) platforms, with a configuration > that enables attestation of the launch measurement. The documentation > for how to actually perform an attestation is severely lacking and > not suitable for mere mortals to understand. IOW, someone trying to > implement attestation is in for a world of pain and suffering. > > This series doesn't fix the documentation problem, but it does > provide a reference implementation of a tool for performing > attestation of SEV(-ES) guests in the context of libvirt / KVM. > > There will be other tools and libraries that implement attestation > logic too, but this tool is likely somewhat unique in its usage of > libvirt. Now for a attestation to be trustworthy you don't want to > perform it on the hypervisor host, since the goal is to prove that > the hypervisor has not acted maliciously. None the less it is still > beneficial to have libvirt integration to some extent. > > When running this tool on a remote (trusted) host, it can connect > to the libvirt hypervisor and fetch the data provided by the > virDomainLaunchSecurityInfo API, which is safe to trust as the > key pieces are cryptographically measured. > > Attestation is a complex problem though and it is very easy to > screw up and feed the wrong information and then waste hours trying > to figure out what piece was wrong, to cause the hash digest to > change. For debugging such problems, you can thus tell the tool > to operate insecurely, by querying libvirt for almost all of the > configuration information required to determine the expected > measurement. By comparing these results,to the results obtained > in offline mode it helps narrow down where the mistake lies. > > So I view this tool as being useful in a number of ways: > > * Quality assurance engineers needing to test libvirt/QEMU/KVM >get a simple and reliable tool for automating tests with. > > * Users running simple libvirt deployments without any large >management stack, get a standalone tool for attestation >they can rely on. > > * Developers writing/integrating attestation support into >management stacks above libvirt, get a reference against >which they can debug their own tools. > > * Users wanting to demonstrate the core SEV/SEV-ES functionality >get a simple and reliable tool to illustrate the core concepts >involved. > > Since I didn't fancy writing such complex logic in C, this tool is > a python3 program. As such, we don't want to include it in the > main libvirt-client RPM, nor any other existing RPM. THus, this > series puts it in a new libvirt-client-qemu RPM which, through no > co-inicidence at all, is the same RPM I invented a few days ago to > hold the virt-qemu-qmp-proxy command. > > Note, people will have already seen an earlier version of this > tool I hacked up some months ago. This code is very significantly > changed since that earlier version, to make it more maintainable, > and simpler to use (especially for SEV-ES) but the general theme > is still the same. > > Daniel P. Berrangé (12): > build-aux: only forbid gethostname in C files > tools: support validating SEV firmware boot measurements > tools: load guest config from libvirt > tools: support validating SEV direct kernel boot measurements > tools: load direct kernel config from libvirt > tools: support validating SEV-ES initial vCPU state measurements > tools: support automatically constructing SEV-ES vCPU state > tools: load CPU count and CPU SKU from libvirt > tools: support generating SEV secret injection tables > docs/kbase: describe attestation for SEV guests > scripts: add systemtap script for capturing SEV-ES VMSA > docs/manpages: add checklist of problems for SEV attestation After the fix I mentioned on patch 7, the script worked for my sev, sev-es, sev-es + kernel setup, with vmsa passed in and vmsa generated. Attached is some pylint output, nothing really serious: - Cole * Module virt-qemu-sev-validate tools/virt-qemu-sev-validate.py:88:35: W0622: Redefining built-in 'format' (redefined-builtin) tools/virt-qemu-sev-validate.py:101:41: W0622: Redefining built-in 'format' (redefined-builtin) tools/virt-qemu-sev-validate.py:151:16: C0200: Consider using enumerate instead of iterating with range and len (consider-using-enumerate) tools/virt-qemu-sev-validate.py:351:8: W0201: Attribute 'cr0' defined outside __init__ (attribute-defined-outside-init) tools/virt-qemu-sev-validate.py:352:8: W0201: Attribute 'rip' defined outside __init__ (attribute-defined-outside-init) tools/virt-qemu-sev-validate.py:354:8: W0201: Attribute 'cs_selector' defined outside __init__ (attribute-defined-outside-init) tools/virt-qemu-sev-validate.py:355:8: W0201: Attribute 'cs_base' defined outside __init__ (attribute-defined-outside-init)
Re: [libvirt PATCH 07/12] tools: support automatically constructing SEV-ES vCPU state
On 10/7/22 7:43 AM, Daniel P. Berrangé wrote:
> The VMSA files contain the expected CPU register state for the VM. Their
> content varies based on a few pieces of the stack
>
> - AMD CPU architectural initial state
> - KVM hypervisor VM CPU initialization
> - QEMU userspace VM CPU initialization
> - AMD CPU SKU (family/model/stepping)
>
> The first three pieces of information we can obtain through code
> inspection. The last piece of information we can take on the command
> line. This allows a user to validate a SEV-ES guest merely by providing
> the CPU SKU information, using --cpu-family, --cpu-model,
> --cpu-stepping. This avoids the need to obtain or construct VMSA files
> directly.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> docs/manpages/virt-qemu-sev-validate.rst | 45 +++
> tools/virt-qemu-sev-validate.py | 475 +++
> 2 files changed, 520 insertions(+)
>
> diff --git a/docs/manpages/virt-qemu-sev-validate.rst
> b/docs/manpages/virt-qemu-sev-validate.rst
> index 24bca98d28..7ba7323e13 100644
> --- a/docs/manpages/virt-qemu-sev-validate.rst
> +++ b/docs/manpages/virt-qemu-sev-validate.rst
> @@ -243,6 +243,24 @@ Validate the measurement of a SEV-ES SMP guest booting
> from disk:
> --build-id 13 \
> --policy 7
>
> +Validate the measurement of a SEV-ES SMP guest booting from disk, with
> +automatically constructed VMSA:
> +
> +::
> +
> + # virt-dom-sev-validate \
> + --firmware OVMF.sev.fd \
> + --num-cpus 2 \
> + --cpu-family 23 \
> + --cpu-model 49 \
> + --cpu-stepping 0 \
> + --tk this-guest-tk.bin \
> + --measurement
> Zs2pf19ubFSafpZ2WKkwquXvACx9Wt/BV+eJwQ/taO8jhyIj/F8swFrybR1fZ2ID \
> + --api-major 0 \
> + --api-minor 24 \
> + --build-id 13 \
> + --policy 7
> +
> Fetch from remote libvirt
> -
>
> @@ -289,6 +307,20 @@ Validate the measurement of a SEV-ES SMP guest booting
> from disk:
> --tk this-guest-tk.bin \
> --domain fedora34x86_64
>
> +Validate the measurement of a SEV-ES SMP guest booting from disk, with
> +automatically constructed VMSA:
> +
> +::
> +
> + # virt-dom-sev-validate \
> + --connect qemu+ssh://r...@some.remote.host/system \
> + --firmware OVMF.sev.fd \
> + --cpu-family 23 \
> + --cpu-model 49 \
> + --cpu-stepping 0 \
> + --tk this-guest-tk.bin \
> + --domain fedora34x86_64
> +
> Fetch from local libvirt
>
>
> @@ -330,6 +362,19 @@ Validate the measurement of a SEV-ES SMP guest booting
> from disk:
> --tk this-guest-tk.bin \
> --domain fedora34x86_64
>
> +Validate the measurement of a SEV-ES SMP guest booting from disk, with
> +automatically constructed VMSA:
> +
> +::
> +
> + # virt-dom-sev-validate \
> + --insecure \
> + --cpu-family 23 \
> + --cpu-model 49 \
> + --cpu-stepping 0 \
> + --tk this-guest-tk.bin \
> + --domain fedora34x86_64
> +
> EXIT STATUS
> ===
>
> diff --git a/tools/virt-qemu-sev-validate.py b/tools/virt-qemu-sev-validate.py
> index ea5be80129..2505aff07f 100755
> --- a/tools/virt-qemu-sev-validate.py
> +++ b/tools/virt-qemu-sev-validate.py
> @@ -44,6 +44,7 @@ import logging
> from lxml import etree
> import re
> import socket
> +from struct import pack
> import sys
> import traceback
> from uuid import UUID
> @@ -71,6 +72,435 @@ class InvalidStateException(Exception):
> pass
>
>
> +class Field(object):
> +U8 = 0
> +U16 = 2
> +U32 = 4
> +U64 = 8
> +
> +SCALAR = 0
> +BITMASK = 1
> +ARRAY = 2
> +
> +def __init__(self, name, size, format, value, order):
> +self.name = name
> +self.size = size
> +self.value = value
> +self.format = format
> +self.order = order
> +
> +
> +class Struct(object):
> +def __init__(self, size):
> +self._fields = {}
> +self.size = size
> +
> +def register_field(self, name, size, format=Field.SCALAR, defvalue=0):
> +self._fields[name] = Field(name, size, format,
> + defvalue, len(self.fields))
> +
> +@property
> +def fields(self):
> +return sorted(self._fields.values(), key=lambda f: f.order)
> +
> +def __getattr__(self, name):
> +return self._fields[name]
> +
> +def __setattr__(self, name, value):
> +if name in ["_fields", "size"]:
> +super().__setattr__(name, value)
> +else:
> +self._fields[name].value = value
> +
> +def binary_format(self):
> +fmt = ["<"]
> +datalen = 0
> +for field in self.fields:
> +if field.size == Field.U8:
> +if field.format == Field.ARRAY:
> +datalen += len(field.value)
> +fmt += ["%dB" % len(field.value)]
> +else:
> +datalen += 1
> +
Re: [libvirt PATCH 02/12] tools: support validating SEV firmware boot measurements
On 10/7/22 7:42 AM, Daniel P. Berrangé wrote:
> The virt-qemu-sev-validate program will compare a reported SEV/SEV-ES
> domain launch measurement, to a computed launch measurement. This
> determines whether the domain has been tampered with during launch.
>
> This initial implementation requires all inputs to be provided
> explicitly, and as such can run completely offline, without any
> connection to libvirt.
>
> The tool is placed in the libvirt-client-qemu sub-RPM since it is
> specific to the QEMU driver.
>
> Signed-off-by: Daniel P. Berrangé
> +try:
> +check_usage(args)
> +
> +attest(args)
> +
> +sys.exit(0)
> +except AttestationFailedException as e:
> +if not args.quiet:
> +print("ERROR: %s" % e, file=sys.stderr)
> +sys.exit(1)
> +except UnsupportedUsageException as e:
> +if not args.quiet:
> +print("ERROR: %s" % e, file=sys.stderr)
> +sys.exit(2)
> +except Exception as e:
> +if args.debug:
> +traceback.print_tb(e.__traceback__)
> +if not args.quiet:
> +print("ERROR: %s" % e, file=sys.stderr)
> +sys.exit(3)
This only tracebacks on --debug for an unexpected error. I think it's
more useful to have --debug always print backtrace. It helped me
debugging usage of the script
Thanks,
Cole
[libvirt PATCH] virvhba.c: use g_autofree
Change strings to use g_autofree.
Signed-off-by: Maxim Kostin
---
src/util/virvhba.c | 21 +++--
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/src/util/virvhba.c b/src/util/virvhba.c
index a4e88024d1..76c39e5f2b 100644
--- a/src/util/virvhba.c
+++ b/src/util/virvhba.c
@@ -49,7 +49,7 @@ bool
virVHBAPathExists(const char *sysfs_prefix,
int host)
{
-char *sysfs_path = NULL;
+g_autofree char *sysfs_path = NULL;
bool ret = false;
sysfs_path = g_strdup_printf("%s/host%d",
@@ -58,7 +58,6 @@ virVHBAPathExists(const char *sysfs_prefix,
if (virFileExists(sysfs_path))
ret = true;
-VIR_FREE(sysfs_path);
return ret;
}
@@ -79,8 +78,8 @@ bool
virVHBAIsVportCapable(const char *sysfs_prefix,
int host)
{
-char *scsi_host_path = NULL;
-char *fc_host_path = NULL;
+g_autofree char *scsi_host_path = NULL;
+g_autofree char *fc_host_path = NULL;
bool ret = false;
fc_host_path = g_strdup_printf("%s/host%d/%s",
@@ -94,8 +93,6 @@ virVHBAIsVportCapable(const char *sysfs_prefix,
if (virFileExists(fc_host_path) || virFileExists(scsi_host_path))
ret = true;
-VIR_FREE(fc_host_path);
-VIR_FREE(scsi_host_path);
return ret;
}
@@ -115,9 +112,9 @@ virVHBAGetConfig(const char *sysfs_prefix,
int host,
const char *entry)
{
-char *sysfs_path = NULL;
+g_autofree char *sysfs_path = NULL;
char *p = NULL;
-char *buf = NULL;
+g_autofree char *buf = NULL;
char *result = NULL;
sysfs_path = g_strdup_printf("%s/host%d/%s",
@@ -140,8 +137,6 @@ virVHBAGetConfig(const char *sysfs_prefix,
result = g_strdup(p);
cleanup:
-VIR_FREE(sysfs_path);
-VIR_FREE(buf);
return result;
}
@@ -315,8 +310,8 @@ vhbaReadCompareWWN(const char *prefix,
const char *f_name,
const char *wwn)
{
-char *path;
-char *buf = NULL;
+g_autofree char *path = NULL;
+g_autofree char *buf = NULL;
char *p;
int ret = -1;
@@ -343,8 +338,6 @@ vhbaReadCompareWWN(const char *prefix,
ret = 1;
cleanup:
-VIR_FREE(path);
-VIR_FREE(buf);
return ret;
}
--
2.34.1
