Accept information about a connection to libvirt and a guest on the
command line. Talk to libvirt to obtain the running guest state and
automatically detect as much configuration as possible.

It will refuse to use a libvirt connection that is thought to be local
to the current machine, as running this tool on the hypervisor itself is
not considered secure. This can be overridden using the --insecure flag.

When querying the guest, it will also analyse the XML configuration in
an attempt to detect any options that are liable to be mistakes. For
example the NVRAM being measured should not have a persistent varstore.

Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
---
 docs/manpages/virt-qemu-sev-validate.rst | 109 +++++++++++-
 tools/virt-qemu-sev-validate.py          | 201 +++++++++++++++++++++--
 2 files changed, 296 insertions(+), 14 deletions(-)

diff --git a/docs/manpages/virt-qemu-sev-validate.rst 
b/docs/manpages/virt-qemu-sev-validate.rst
index 36de9becfd..2c02a27103 100644
--- a/docs/manpages/virt-qemu-sev-validate.rst
+++ b/docs/manpages/virt-qemu-sev-validate.rst
@@ -37,6 +37,12 @@ another virtual machine protected by AMD SEV that has 
already had its launch
 measurement validated. Running this program on the virtualization host will not
 produce an answer that can be trusted.
 
+If told to connect to libvirt, it will refuse to use a libvirt connection that
+is local to the machine, since that cannot be trusted. For the sake of testing
+or demonstration purposes, however, it can be forced to run in this scenario
+using the ``--insecure`` flag. The result will, of course, still not be
+trustworthy.
+
 OPTIONS
 =======
 
@@ -115,6 +121,41 @@ size, with the first 16 bytes containing the TEK and the 
last 16 bytes
 containing the TIK.  This is mutually exclusive with the ``--tik`` and 
``--tek``
 arguments.
 
+Libvirt options
+---------------
+
+These options are used when connecting to libvirt to automatically obtain
+state and configuration information about the domain to be attested.
+
+``-c``, ``--connect URI``
+
+Libvirt connection URI. For the validation to be trustworthy this must be a URI
+resolving to a remote virtualization host. This requirement can be overridden
+using the ``--insecure`` argument
+
+``-o``, ``--domain ID|NAME|UUID``
+
+Domain ID, or domain name or domain UUID. Used to identify which libvirt domain
+is to have its launch measured. The domain must be running, and would usually
+have been started in a paused state, to allow validation to be performed before
+guest CPUs begin execution.
+
+``-i``, ``--insecure``
+
+Proceed even if usage scenario is known to be insecure. This allows the program
+to connect to a local libvirt hypervisor and rely on file content from the
+virtualization host. The result of the validation must not be trusted.
+
+``-g``, ``--ignore-config``
+
+Do not attempt to sanity check the domain config. The default behaviour is to
+print out errors if identifying configuration elements in the guest XML that
+would invalidate the launch measurement. This can help the guest owner to
+understand any configuration mistakes that have been made. If the
+``--ignore-config`` argument is given, this sanity checking of configuration
+will be skipped. The result is that the validation will likely be reported as
+failed.
+
 EXAMPLES
 ========
 
@@ -139,6 +180,46 @@ Validate the measurement of a SEV guest booting from disk:
        --build-id 13 \
        --policy 3
 
+Fetch from remote libvirt
+-------------------------
+
+This scenario allows fetching certain data from a remote hypervisor via a
+connection to libvirt. It will aid in debugging by analysing the guest
+configuration and reporting anything that could invalidate the measurement
+of the guest. This usage model is considered secure, because the limited
+information obtained from the untrusted hypervisor cannot be used to change
+the result.
+
+Validate the measurement of a SEV guest booting from disk:
+
+::
+
+   # virt-qemu-sev-validate \
+       --connect qemu+ssh://r...@some.remote.host/system \
+       --firmware OVMF.sev.fd \
+       --tk this-guest-tk.bin \
+       --domain fedora34x86_64
+
+Fetch from local libvirt
+------------------------
+
+This scenario allows fetching all data from the local hypervisor via a
+connection to libvirt. It is only to be used for the purpose of testing,
+debugging, or demonstrations, because running on the local hypervisor is not
+a secure scenario. To enable this usage, the ``--insecure`` flag must be
+specified. Given a pointer to the libvirt guest to validate, all information
+needed to perform a validation, except the TIK/TEK pair can be acquired
+automatically.
+
+Validate the measurement of a SEV guest booting from disk:
+
+::
+
+   # virt-qemu-sev-validate \
+       --insecure \
+       --tk this-guest-tk.bin \
+       --domain fedora34x86_64
+
 EXIT STATUS
 ===========
 
@@ -158,7 +239,33 @@ be set:
   The way in which this program has been invoked prevent it from being able to
   validate the launch measurement.
 
-* **3** - *unexpected error occurred in the code*
+* **3** - *Usage scenario is not secure*
+
+  The way in which this program has been invoked means that the result of any
+  launch measurement validation will not be secure.
+
+  The program can be reinvoked with ``--insecure`` argument to force a
+  validation, however, the results of this should not be trusted. This should
+  only be used for testing, debugging or demonstration purposes, never in a
+  production deployment.
+
+* **4** - *Domain has incorrect configuration to be measured*
+
+  The way in which the guest has been configured prevent this program from 
being
+  able to validate the launch measurement. Note that in general the guest
+  configuration reported by the hypervisor is not trustworthy, so it is
+  possible this error could be a false positive designed to cause a denial of
+  service.
+
+  This program can be reinvoked with the ``--ignore-config`` argument to skip
+  the sanity checks on the domain XML. This will likely result in it failing
+  with an exit code of **1** indicating the measurement is invalid
+
+* **5** - *Domain is in incorrect state to be measured*
+
+  The domain has to be running in order to validate a launch measurement.
+
+* **6** - *unexpected error occurred in the code*
 
   A logic flaw in this program means it is unable to complete the validation of
   the measurement. This is a bug which should be reported to the maintainers.
diff --git a/tools/virt-qemu-sev-validate.py b/tools/virt-qemu-sev-validate.py
index e206b3befa..c47e565224 100755
--- a/tools/virt-qemu-sev-validate.py
+++ b/tools/virt-qemu-sev-validate.py
@@ -38,7 +38,11 @@ import argparse
 from base64 import b64decode
 from hashlib import sha256
 import hmac
+import libvirt
 import logging
+from lxml import etree
+import re
+import socket
 import sys
 import traceback
 
@@ -53,6 +57,18 @@ class UnsupportedUsageException(Exception):
     pass
 
 
+class InsecureUsageException(Exception):
+    pass
+
+
+class IncorrectConfigException(Exception):
+    pass
+
+
+class InvalidStateException(Exception):
+    pass
+
+
 class ConfidentialVM(object):
 
     def __init__(self,
@@ -159,6 +175,103 @@ class ConfidentialVM(object):
                 "Measurement does not match, VM is not trustworthy")
 
 
+class LibvirtConfidentialVM(ConfidentialVM):
+    def __init__(self, **kwargs):
+        super().__init__(**kwargs)
+
+        self.conn = None
+        self.dom = None
+
+    def check_domain(self, doc, secure):
+        ls = doc.xpath("/domain/launchSecurity[@type='sev']")
+        if len(ls) != 1:
+            raise IncorrectConfigException(
+                "Domain is not configured with SEV launch security")
+
+        dh = doc.xpath("/domain/launchSecurity[@type='sev']/dhCert")
+        if len(dh) != 1:
+            raise IncorrectConfigException(
+                "Domain must have SEV owner cert to validate measurement")
+
+        session = doc.xpath("/domain/launchSecurity[@type='sev']/session")
+        if len(session) != 1:
+            raise IncorrectConfigException(
+                "Domain must have SEV session data to validate measurement")
+
+        nvramnodes = doc.xpath("/domain/os/nvram")
+        if len(nvramnodes) != 0 and secure:
+            raise InsecureUsageException(
+                "Domain firmware with NVRAM cannot be securely measured")
+
+        loadernodes = doc.xpath("/domain/os/loader")
+        if len(loadernodes) != 1:
+            raise IncorrectConfigException(
+                "Domain must have one firmware path")
+
+    def load_domain(self, uri, id_name_uuid, secure, ignore_config):
+        self.conn = libvirt.open(uri)
+
+        remote = socket.gethostname() != self.conn.getHostname()
+        if not remote and secure:
+            raise InsecureUsageException(
+                "running locally on the hypervisor host is not secure")
+
+        if re.match(r'^\d+$', id_name_uuid):
+            self.dom = self.conn.lookupByID(int(id_name_uuid))
+        elif re.match(r'^[-a-f0-9]+$', id_name_uuid):
+            self.dom = self.conn.lookupByUUIDString(id_name_uuid)
+        else:
+            self.dom = self.conn.lookupByName(id_name_uuid)
+
+        log.debug("VM: id=%d name=%s uuid=%s" % (
+            self.dom.ID(), self.dom.name(), self.dom.UUIDString()))
+
+        if not self.dom.isActive():
+            raise InvalidStateException(
+                "Domain must be running to validate measurement")
+
+        xml = self.dom.XMLDesc()
+
+        doc = etree.fromstring(xml)
+        if not ignore_config:
+            self.check_domain(doc, secure)
+
+        # See comments at top of file wrt why we are OK to trust the
+        # sev-api-major, sev-api-minor, sev-build-id and sev-policy data
+        # reported by the host
+        sevinfo = self.dom.launchSecurityInfo()
+
+        if "sev-api-major" not in sevinfo:
+            raise UnsupportedUsageException(
+                "'api-major' not reported in domain launch security info")
+
+        if self.measurement is None:
+            self.measurement = sevinfo["sev-measurement"]
+        if self.api_major is None:
+            self.api_major = sevinfo["sev-api-major"]
+        if self.api_minor is None:
+            self.api_minor = sevinfo["sev-api-minor"]
+        if self.build_id is None:
+            self.build_id = sevinfo["sev-build-id"]
+        if self.policy is None:
+            self.policy = sevinfo["sev-policy"]
+
+        if self.firmware is None:
+            if remote:
+                raise UnsupportedUsageException(
+                    "Cannot access loader path remotely")
+            if secure:
+                raise InsecureUsageException(
+                    "Using loader path from XML is not secure")
+
+            loadernodes = doc.xpath("/domain/os/loader")
+            if len(loadernodes) == 0:
+                raise UnsupportedUsageException(
+                    "--firmware not specified and not loader path found")
+
+            self.load_firmware(loadernodes[0].text)
+
+
 def parse_command_line():
     parser = argparse.ArgumentParser(
         description='Validate guest AMD SEV launch measurement')
@@ -169,20 +282,20 @@ def parse_command_line():
 
     # Arguments related to the state of the launched guest
     vmstate = parser.add_argument_group("Virtual machine launch state")
-    vmstate.add_argument('--measurement', '-m', required=True,
+    vmstate.add_argument('--measurement', '-m',
                          help='Measurement for the running domain')
-    vmstate.add_argument('--api-major', type=int, required=True,
+    vmstate.add_argument('--api-major', type=int,
                          help='SEV API major version for the running domain')
-    vmstate.add_argument('--api-minor', type=int, required=True,
+    vmstate.add_argument('--api-minor', type=int,
                          help='SEV API major version for the running domain')
-    vmstate.add_argument('--build-id', type=int, required=True,
+    vmstate.add_argument('--build-id', type=int,
                          help='SEV build ID for the running domain')
-    vmstate.add_argument('--policy', type=int, required=True,
+    vmstate.add_argument('--policy', type=int,
                          help='SEV policy for the running domain')
 
     # Arguments related to calculation of the expected launch measurement
     vmconfig = parser.add_argument_group("Virtual machine config")
-    vmconfig.add_argument('--firmware', '-f', required=True,
+    vmconfig.add_argument('--firmware', '-f',
                           help='Path to the firmware binary')
     vmconfig.add_argument('--tik',
                           help='TIK file for domain')
@@ -191,6 +304,17 @@ def parse_command_line():
     vmconfig.add_argument('--tk',
                           help='TEK/TIK combined file for domain')
 
+    # Arguments related to the connection to libvirt
+    vmconn = parser.add_argument_group("Libvirt guest connection")
+    vmconn.add_argument('--connect', '-c', default="qemu:///system",
+                        help='libvirt connection URI')
+    vmconn.add_argument('--domain', '-o',
+                        help='domain ID / Name / UUID')
+    vmconn.add_argument('--insecure', '-i', action='store_true',
+                        help='Proceed even if usage scenario is insecure')
+    vmconn.add_argument('--ignore-config', '-g', action='store_true',
+                        help='Do not attempt to sanity check the guest config')
+
     return parser.parse_args()
 
 
@@ -206,21 +330,60 @@ def check_usage(args):
             raise UnsupportedUsageException(
                 "Either --tk or both of --tek/--tik are required")
 
+    if args.domain is None:
+        if args.measurement is None:
+            raise UnsupportedUsageException(
+                "Either --measurement or --domain is required")
+
+        if args.api_major is None:
+            raise UnsupportedUsageException(
+                "Either --api-major or --domain is required")
+
+        if args.api_minor is None:
+            raise UnsupportedUsageException(
+                "Either --api-minor or --domain is required")
+
+        if args.build_id is None:
+            raise UnsupportedUsageException(
+                "Either --build-id or --domain is required")
+
+        if args.policy is None:
+            raise UnsupportedUsageException(
+                "Either --policy or --domain is required")
+
+        if args.firmware is None:
+            raise UnsupportedUsageException(
+                "Either --firmware or --domain is required")
+
 
 def attest(args):
-    cvm = ConfidentialVM(measurement=args.measurement,
-                         api_major=args.api_major,
-                         api_minor=args.api_minor,
-                         build_id=args.build_id,
-                         policy=args.policy)
+    if args.domain is None:
+        cvm = ConfidentialVM(measurement=args.measurement,
+                             api_major=args.api_major,
+                             api_minor=args.api_minor,
+                             build_id=args.build_id,
+                             policy=args.policy)
+    else:
+        cvm = LibvirtConfidentialVM(measurement=args.measurement,
+                                    api_major=args.api_major,
+                                    api_minor=args.api_minor,
+                                    build_id=args.build_id,
+                                    policy=args.policy)
 
-    cvm.load_firmware(args.firmware)
+    if args.firmware is not None:
+        cvm.load_firmware(args.firmware)
 
     if args.tk is not None:
         cvm.load_tk(args.tk)
     else:
         cvm.load_tik_tek(args.tik, args.tek)
 
+    if args.domain is not None:
+        cvm.load_domain(args.connect,
+                        args.domain,
+                        not args.insecure,
+                        args.ignore_config)
+
     cvm.attest()
 
     if not args.quiet:
@@ -249,9 +412,21 @@ if __name__ == "__main__":
         if not args.quiet:
             print("ERROR: %s" % e, file=sys.stderr)
         sys.exit(2)
+    except InsecureUsageException as e:
+        if not args.quiet:
+            print("ERROR: %s" % e, file=sys.stderr)
+        sys.exit(3)
+    except IncorrectConfigException as e:
+        if not args.quiet:
+            print("ERROR: %s" % e, file=sys.stderr)
+        sys.exit(4)
+    except InvalidStateException as e:
+        if not args.quiet:
+            print("ERROR: %s" % e, file=sys.stderr)
+        sys.exit(5)
     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)
+        sys.exit(6)
-- 
2.37.3

Reply via email to