[libvirt] [PATCH 2/6] tpm: Add support for external swtpm TPM emulator

Stefan Berger posted 6 patches 7 years, 1 month ago
There is a newer version of this series
[libvirt] [PATCH 2/6] tpm: Add support for external swtpm TPM emulator
Posted by Stefan Berger 7 years, 1 month ago
This patch adds support for an external swtpm TPM emulator. The XML for
this type of TPM looks as follows:

 <tpm model='tpm-tis'>
   <backend type='emulator'/>
 </tpm>

The XML will currently only start a TPM 1.2.

Upon the first start, libvirt will run `swtpm_setup`, which will simulate the
manufacturing of a TPM and create certificates for it and write them into the
NVRAM location of the emulated TPM.

Then, libvirt will automatically start the swtpm TPM emulator using the `swtpm`
executable.

Once the VM terminates, libvirt uses the swtpm_ioctl executable to gracefully
shut down the `swtpm` in case it is still running (QEMU did not send shutdown)
or clean up the socket file.

The above mentioned executables must be found in the PATH.

The executables can either be run as root or started as root and switch to
the tss user. The requirement for the tss user comes through 'tcsd', which
is used for the simulation of the manufacturing. Which user is used can be
configured through qemu.conf.

The swtpm writes out state into files. The state is kept in /var/lib/libvirt/tpm:

[root@localhost libvirt]# ls -lZ | grep tpm

drwx--x--x. 7 root root unconfined_u:object_r:virt_var_lib_t:s0 4096 Apr  5 16:22 tpm

The directory /var/lib/libvirt/tpm maintains per-TPM state directories but
also hosts the UnixIO socket of running swtpms, which QEMU uses for communicating
with them. At this point only the socket file is labeled properly and made accessible
for QEMU, which runs under the qemu user:

[root@localhost tpm]# ls -lZ
total 4
drwx------. 2 tss  tss  system_u:object_r:virt_var_lib_t:s0          4096 Apr  5 16:46 485d0004-a48f-436a-8457-8a3b73e28567
srw-------. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c413,c430    0 Apr  5 16:46 485d0004-a48f-436a-8457-8a3b73e28567.sock

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ls -lZ
total 8
-rw-r--r--. 1 tss tss system_u:object_r:virt_var_lib_t:s0 3648 Apr  5 16:46 tpm-00.permall
-rw-r--r--. 1 tss tss system_u:object_r:virt_var_lib_t:s0 2237 Apr  5 16:46 vtpm.log

root@sbct-3 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep -v grep
system_u:system_r:virtd_t:s0-s0:c0.c1023 tss 18697 0.0  0.0 28172 3892 ?       Ss   16:46   0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567.sock,mode=0600 --tpmstate dir=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567 --log file=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567/vtpm.log --runas 59

[root@sbct-3 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep
system_u:system_r:svirt_t:s0:c413,c430 qemu 18702 2.5  0.0 3036052 48676 ?     Sl   16:46   0:08 /bin/qemu-system-x86_64 -name guest=centos7.0,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-6-centos7.0/master-key.aes -machine pc-i440fx-2.8,accel=kvm,usb=off,dump-guest-core=off -cpu kvm64 -m 2048 -realtime mlock=off -smp 2,sockets=2,cores=1,threads=1 -uuid 485d0004-a48f-436a-8457-8a3b73e28567 [...] -tpmdev emulator,id=tpm-tpm0,chardev=chrtpm -chardev socket,id=chrtpm,path=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567.sock -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 -device usb-mouse,id=input0,bus=usb.0,port=1 -vnc 127.0.0.1:0 -device cirrus-vga,id=video0,bus=pci.0,addr=0x2 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 -msg timestamp=on

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 docs/formatdomain.html.in                          |  30 ++
 docs/schemas/domaincommon.rng                      |   5 +
 src/conf/domain_audit.c                            |   2 +
 src/conf/domain_conf.c                             |  51 ++-
 src/conf/domain_conf.h                             |   5 +
 src/libvirt_private.syms                           |   5 +
 src/qemu/Makefile.inc.am                           |   2 +
 src/qemu/libvirtd_qemu.aug                         |   3 +
 src/qemu/qemu.conf                                 |   7 +
 src/qemu/qemu_capabilities.c                       |   5 +
 src/qemu/qemu_capabilities.h                       |   1 +
 src/qemu/qemu_cgroup.c                             |   1 +
 src/qemu/qemu_command.c                            |  52 ++-
 src/qemu/qemu_conf.c                               |  11 +-
 src/qemu/qemu_conf.h                               |   2 +
 src/qemu/qemu_domain.c                             |   2 +
 src/qemu/qemu_driver.c                             |  13 +
 src/qemu/qemu_extdevice.c                          | 195 ++++++++++
 src/qemu/qemu_extdevice.h                          |  36 ++
 src/qemu/qemu_process.c                            |   8 +
 src/qemu/test_libvirtd_qemu.aug.in                 |   1 +
 src/security/security_dac.c                        |   6 +
 src/security/security_selinux.c                    |  11 +
 src/util/virfile.c                                 |  12 +
 src/util/virfile.h                                 |   2 +-
 src/util/virtpm.c                                  | 432 +++++++++++++++++++++
 src/util/virtpm.h                                  |  12 +
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   |   1 +
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml |   1 +
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |   1 +
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   |   1 +
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   1 +
 tests/qemuxml2argvdata/tpm-emulator.args           |  24 ++
 tests/qemuxml2argvdata/tpm-emulator.xml            |  30 ++
 tests/qemuxml2argvmock.c                           |   2 +
 tests/qemuxml2argvtest.c                           |  17 +
 tests/qemuxml2xmloutdata/tpm-emulator.xml          |  34 ++
 tests/qemuxml2xmltest.c                            |   1 +
 38 files changed, 1011 insertions(+), 14 deletions(-)
 create mode 100644 src/qemu/qemu_extdevice.c
 create mode 100644 src/qemu/qemu_extdevice.h
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator.args
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml
 create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 16fc7db..bd6fedc 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7621,6 +7621,26 @@ qemu-kvm -net nic,model=? /dev/null
 &lt;/devices&gt;
 ...
 </pre>
+
+    <p>
+      The emulator device type gives access to a TPM emulator providing
+      TPM functionlity for each VM. QEMU talks to it over a UnixIO socket. With
+      the emulator device type each guest gets its own private TPM.
+      <span class="since">'emulator' since 4.x.y</span>
+    </p>
+    <p>
+     Example: usage of the TPM Emulator
+    </p>
+<pre>
+  ...
+  &lt;devices&gt;
+    &lt;tpm model='tpm-tis'&gt;
+      &lt;backend type='emulator'&gt;
+      &lt;/backend&gt;
+    &lt;/tpm&gt;
+  &lt;/devices&gt;
+  ...
+</pre>
     <dl>
       <dt><code>model</code></dt>
       <dd>
@@ -7653,6 +7673,16 @@ qemu-kvm -net nic,model=? /dev/null
             </p>
           </dd>
         </dl>
+        <dl>
+          <dt><code>emulator</code></dt>
+          <dd>
+            <p>
+              For this backend type the 'swtpm' TPM Emulator must be installed on the
+              host. Libvirt will automatically start an independent TPM emulator
+              for each QEMU guest requesting access to it.
+            </p>
+          </dd>
+        </dl>
       </dd>
     </dl>
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index be5c628..d628444 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4134,6 +4134,11 @@
           </attribute>
           <ref name="tpm-passthrough-device"/>
         </group>
+        <group>
+          <attribute name="type">
+             <value>emulator</value>
+          </attribute>
+        </group>
       </choice>
     </element>
   </define>
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 82868bc..25cccdd 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -586,6 +586,8 @@ virDomainAuditTPM(virDomainObjPtr vm, virDomainTPMDefPtr tpm,
                   "virt=%s resrc=dev reason=%s %s uuid=%s %s",
                   virt, reason, vmname, uuidstr, device);
         break;
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
     default:
         break;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 232174a..da14ef8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -862,7 +862,8 @@ VIR_ENUM_IMPL(virDomainTPMModel, VIR_DOMAIN_TPM_MODEL_LAST,
               "tpm-crb")
 
 VIR_ENUM_IMPL(virDomainTPMBackend, VIR_DOMAIN_TPM_TYPE_LAST,
-              "passthrough")
+              "passthrough",
+              "emulator")
 
 VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST,
               "intel")
@@ -2588,6 +2589,29 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
     }
 }
 
+static void virDomainTPMDeleteAny(const virDomainDef *def)
+{
+    virTPMDeleteEmulatorStorage(def->uuid);
+}
+
+void virDomainTPMDelete(virDomainDefPtr def)
+{
+    virDomainTPMDefPtr tpm = def->tpm;
+
+    if (!tpm)
+        return;
+
+    switch (tpm->type) {
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        virTPMDeleteEmulatorStorage(def->uuid);
+        break;
+    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+    case VIR_DOMAIN_TPM_TYPE_LAST:
+        /* nothing to do */
+        break;
+    }
+}
+
 void virDomainTPMDefFree(virDomainTPMDefPtr def)
 {
     if (!def)
@@ -2597,6 +2621,9 @@ void virDomainTPMDefFree(virDomainTPMDefPtr def)
     case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
         VIR_FREE(def->data.passthrough.source.data.file.path);
         break;
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        VIR_FREE(def->data.emulator.source.data.nix.path);
+        break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
         break;
     }
@@ -12525,6 +12552,11 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
  *   </backend>
  * </tpm>
  *
+ * or like this:
+ *
+ * <tpm model='tpm-tis'>
+ *   <backend type='emulator'/>
+ * </tpm>
  */
 static virDomainTPMDefPtr
 virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
@@ -12591,6 +12623,8 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
         def->data.passthrough.source.type = VIR_DOMAIN_CHR_TYPE_DEV;
         path = NULL;
         break;
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
         goto error;
     }
@@ -24760,24 +24794,32 @@ virDomainTPMDefFormat(virBufferPtr buf,
                       virDomainTPMDefPtr def,
                       unsigned int flags)
 {
+    bool did_nl = false;
+
     virBufferAsprintf(buf, "<tpm model='%s'>\n",
                       virDomainTPMModelTypeToString(def->model));
     virBufferAdjustIndent(buf, 2);
-    virBufferAsprintf(buf, "<backend type='%s'>\n",
+    virBufferAsprintf(buf, "<backend type='%s'",
                       virDomainTPMBackendTypeToString(def->type));
     virBufferAdjustIndent(buf, 2);
 
     switch (def->type) {
     case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+        virBufferAddLit(buf, ">\n");
+        did_nl = true;
         virBufferEscapeString(buf, "<device path='%s'/>\n",
                               def->data.passthrough.source.data.file.path);
         break;
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
     case VIR_DOMAIN_TPM_TYPE_LAST:
         break;
     }
 
     virBufferAdjustIndent(buf, -2);
-    virBufferAddLit(buf, "</backend>\n");
+    if (did_nl)
+        virBufferAddLit(buf, "</backend>\n");
+    else
+        virBufferAddLit(buf, "/>\n");
 
     virDomainDeviceInfoFormat(buf, &def->info, flags);
 
@@ -27548,6 +27590,9 @@ virDomainDeleteConfig(const char *configDir,
         goto cleanup;
     }
 
+    /* in case it had a TPM, remove it */
+    virDomainTPMDeleteAny(dom->def);
+
     ret = 0;
 
  cleanup:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1724340..4ecc70d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1284,6 +1284,7 @@ typedef enum {
 
 typedef enum {
     VIR_DOMAIN_TPM_TYPE_PASSTHROUGH,
+    VIR_DOMAIN_TPM_TYPE_EMULATOR,
 
     VIR_DOMAIN_TPM_TYPE_LAST
 } virDomainTPMBackendType;
@@ -1298,6 +1299,9 @@ struct _virDomainTPMDef {
         struct {
             virDomainChrSourceDef source;
         } passthrough;
+        struct {
+            virDomainChrSourceDef source;
+        } emulator;
     } data;
 };
 
@@ -2810,6 +2814,7 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
                                   int type);
 virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device);
 void virDomainTPMDefFree(virDomainTPMDefPtr def);
+void virDomainTPMDelete(virDomainDefPtr def);
 
 typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def,
                                            virDomainDeviceDefPtr dev,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 03fe3b3..e64bbef 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -556,6 +556,7 @@ virDomainTimerTrackTypeToString;
 virDomainTPMBackendTypeFromString;
 virDomainTPMBackendTypeToString;
 virDomainTPMDefFree;
+virDomainTPMDelete;
 virDomainTPMModelTypeFromString;
 virDomainTPMModelTypeToString;
 virDomainUSBDeviceDefForeach;
@@ -2971,6 +2972,10 @@ virTimeStringThenRaw;
 
 # util/virtpm.h
 virTPMCreateCancelPath;
+virTPMDeleteEmulatorStorage;
+virTPMEmulatorBuildCommand;
+virTPMStopEmulator;
+virTPMTryConnect;
 
 
 # util/virtypedparam.h
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 8ef290a..6c8daf8 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -19,6 +19,8 @@ QEMU_DRIVER_SOURCES = \
 	qemu/qemu_domain_address.h \
 	qemu/qemu_cgroup.c \
 	qemu/qemu_cgroup.h \
+	qemu/qemu_extdevice.c \
+	qemu/qemu_extdevice.h \
 	qemu/qemu_hostdev.c \
 	qemu/qemu_hostdev.h \
 	qemu/qemu_hotplug.c \
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index c19bf3a..cc5d657 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -118,6 +118,8 @@ module Libvirtd_qemu =
    let vxhs_entry = bool_entry "vxhs_tls"
                  | str_entry "vxhs_tls_x509_cert_dir"
 
+   let swtpm_entry = str_entry "swtpm_user"
+
    (* Each entry in the config is one of the following ... *)
    let entry = default_tls_entry
              | vnc_entry
@@ -137,6 +139,7 @@ module Libvirtd_qemu =
              | gluster_debug_level_entry
              | memory_entry
              | vxhs_entry
+             | swtpm_entry
 
    let comment = [ label "#comment" . del /#[ \t]*/ "# " .  store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ]
    let empty = [ label "#empty" . eol ]
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 43dd561..f64ae68 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -775,3 +775,10 @@
 # This directory is used for memoryBacking source if configured as file.
 # NOTE: big files will be stored here
 #memory_backing_dir = "/var/lib/libvirt/qemu/ram"
+
+# User for the swtpm TPM Emulator
+#
+# Default is 'tss'; this is the same user that tcsd (TrouSerS) installs
+# and uses; alternative is 'root'
+#
+#swtpm_user = "tss"
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0952663..ce4db62 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -467,6 +467,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
               "virtio-mouse-ccw",
               "virtio-tablet-ccw",
               "tpm-crb",
+              "tpm-emulator",
     );
 
 
@@ -3098,6 +3099,10 @@ static const struct tpmTypeToCaps virQEMUCapsTPMTypesToCaps[] = {
         .type = VIR_DOMAIN_TPM_TYPE_PASSTHROUGH,
         .caps = QEMU_CAPS_DEVICE_TPM_PASSTHROUGH,
     },
+    {
+        .type = VIR_DOMAIN_TPM_TYPE_EMULATOR,
+        .caps = QEMU_CAPS_DEVICE_TPM_EMULATOR,
+    },
 };
 
 const struct tpmTypeToCaps virQEMUCapsTPMModelsToCaps[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 604525a..0cc2882 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -451,6 +451,7 @@ typedef enum {
     QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW, /* -device virtio-mouse-ccw */
     QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW, /* -device virtio-tablet-ccw */
     QEMU_CAPS_DEVICE_TPM_CRB, /* -device tpm-crb */
+    QEMU_CAPS_DEVICE_TPM_EMULATOR, /* -tpmdev emulator */
 
     QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index b604edb..bd4859c 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -238,6 +238,7 @@ qemuSetupTPMCgroup(virDomainObjPtr vm)
     case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
         ret = qemuSetupChrSourceCgroup(vm, &dev->data.passthrough.source);
         break;
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
     case VIR_DOMAIN_TPM_TYPE_LAST:
         break;
     }
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 89fd08b..878a147 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9614,21 +9614,33 @@ qemuBuildTPMDevStr(const virDomainDef *def,
 
 
 static char *
-qemuBuildTPMBackendStr(const virDomainDef *def,
+qemuBuildTPMBackendStr(virDomainDef *def,
+                       virQEMUDriverPtr driver,
                        virCommandPtr cmd,
                        virQEMUCapsPtr qemuCaps,
                        int *tpmfd,
-                       int *cancelfd)
+                       int *cancelfd,
+                       char **chardev)
 {
-    const virDomainTPMDef *tpm = def->tpm;
+    virDomainTPMDef *tpm = def->tpm;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
-    const char *type = virDomainTPMBackendTypeToString(tpm->type);
+    const char *type = NULL;
     char *cancel_path = NULL, *devset = NULL;
     const char *tpmdev;
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 
     *tpmfd = -1;
     *cancelfd = -1;
 
+    switch (tpm->type) {
+    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        type = virDomainTPMBackendTypeToString(tpm->type);
+        break;
+    case VIR_DOMAIN_TPM_TYPE_LAST:
+        goto error;
+    }
+
     virBufferAsprintf(&buf, "%s,id=tpm-%s", type, tpm->info.alias);
 
     switch (tpm->type) {
@@ -9679,6 +9691,17 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
         VIR_FREE(cancel_path);
 
         break;
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_EMULATOR))
+            goto no_support;
+
+        virBufferAddLit(&buf, ",chardev=chrtpm");
+
+        if (virAsprintf(chardev, "socket,id=chrtpm,path=%s",
+                        tpm->data.emulator.source.data.nix.path) < 0)
+            goto error;
+
+        break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
         goto error;
     }
@@ -9686,6 +9709,8 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
     if (virBufferCheckError(&buf) < 0)
         goto error;
 
+     virObjectUnref(cfg);
+
     return virBufferContentAndReset(&buf);
 
  no_support:
@@ -9699,16 +9724,19 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
     VIR_FREE(cancel_path);
 
     virBufferFreeAndReset(&buf);
+    virObjectUnref(cfg);
     return NULL;
 }
 
 
 static int
-qemuBuildTPMCommandLine(virCommandPtr cmd,
-                        const virDomainDef *def,
+qemuBuildTPMCommandLine(virQEMUDriverPtr driver,
+                        virCommandPtr cmd,
+                        virDomainDef *def,
                         virQEMUCapsPtr qemuCaps)
 {
     char *optstr;
+    char *chardev = NULL;
     int tpmfd = -1;
     int cancelfd = -1;
     char *fdset;
@@ -9716,13 +9744,19 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
     if (!def->tpm)
         return 0;
 
-    if (!(optstr = qemuBuildTPMBackendStr(def, cmd, qemuCaps,
-                                          &tpmfd, &cancelfd)))
+    if (!(optstr = qemuBuildTPMBackendStr(def, driver, cmd, qemuCaps,
+                                          &tpmfd, &cancelfd,
+                                          &chardev)))
         return -1;
 
     virCommandAddArgList(cmd, "-tpmdev", optstr, NULL);
     VIR_FREE(optstr);
 
+    if (chardev) {
+        virCommandAddArgList(cmd, "-chardev", chardev, NULL);
+        VIR_FREE(chardev);
+    }
+
     if (tpmfd >= 0) {
         fdset = qemuVirCommandGetFDSet(cmd, tpmfd);
         if (!fdset)
@@ -10151,7 +10185,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
                                     chardevStdioLogd) < 0)
         goto error;
 
-    if (qemuBuildTPMCommandLine(cmd, def, qemuCaps) < 0)
+    if (qemuBuildTPMCommandLine(driver, cmd, def, qemuCaps) < 0)
         goto error;
 
     if (qemuBuildInputCommandLine(cmd, def, qemuCaps) < 0)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 36cf3a2..a204105 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -336,6 +336,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
                              &cfg->nfirmwares) < 0)
         goto error;
 
+    if (virGetUserID("tss", &cfg->swtpm_user) < 0)
+        cfg->swtpm_user = 0; /* root */
+
     return cfg;
 
  error:
@@ -475,7 +478,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
     int rv;
     size_t i, j;
     char *stdioHandler = NULL;
-    char *user = NULL, *group = NULL;
+    char *user = NULL, *group = NULL, *swtpm_user = NULL;
     char **controllers = NULL;
     char **hugetlbfs = NULL;
     char **nvram = NULL;
@@ -912,6 +915,11 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
     if (virConfGetValueString(conf, "memory_backing_dir", &cfg->memoryBackingDir) < 0)
         goto cleanup;
 
+    if (virConfGetValueString(conf, "swtpm_user", &swtpm_user) < 0)
+        goto cleanup;
+    if (swtpm_user && virGetUserID(swtpm_user, &cfg->swtpm_user) < 0)
+        goto cleanup;
+
     ret = 0;
 
  cleanup:
@@ -922,6 +930,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
     VIR_FREE(corestr);
     VIR_FREE(user);
     VIR_FREE(group);
+    VIR_FREE(swtpm_user);
     virConfFree(conf);
     return ret;
 }
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index e1ad546..6908c36 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -206,6 +206,8 @@ struct _virQEMUDriverConfig {
 
     bool vxhsTLS;
     char *vxhsTLSx509certdir;
+
+    uid_t swtpm_user;
 };
 
 /* Main driver state */
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 580e0f8..9b7f8ff 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7088,6 +7088,7 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver,
             VIR_WARN("unable to remove snapshot directory %s", snapDir);
         VIR_FREE(snapDir);
     }
+    virDomainTPMDelete(vm->def);
 
     virObjectRef(vm);
 
@@ -10280,6 +10281,7 @@ qemuDomainSetupTPM(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
             return -1;
         break;
 
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
     case VIR_DOMAIN_TPM_TYPE_LAST:
         /* nada */
         break;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7bcc493..ef0d0c9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14365,6 +14365,19 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
         goto cleanup;
     }
 
+    if (vm->def->tpm) {
+        switch (vm->def->tpm->type) {
+        case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("attached '%s' TPM does not allow snapshots"),
+                           virDomainTPMBackendTypeToString(vm->def->tpm->type));
+            goto cleanup;
+        case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        case VIR_DOMAIN_TPM_TYPE_LAST:
+        break;
+        }
+    }
+
     for (i = 0; i < def->ndisks; i++) {
         virDomainSnapshotDiskDefPtr disk = &def->disks[i];
         virDomainDiskDefPtr dom_disk = vm->def->disks[i];
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
new file mode 100644
index 0000000..4f42c9b
--- /dev/null
+++ b/src/qemu/qemu_extdevice.c
@@ -0,0 +1,195 @@
+/*
+ * qemu_extdevice.c: QEMU external devices support
+ *
+ * Copyright (C) 2014 IBM Corporation
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Stefan Berger <stefanb@linux.vnet.ibm.com>
+ */
+
+#include <config.h>
+
+#include "qemu_extdevice.h"
+#include "qemu_domain.h"
+
+#include "viralloc.h"
+#include "virlog.h"
+#include "virstring.h"
+#include "virtime.h"
+#include "virtpm.h"
+
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+VIR_LOG_INIT("qemu.qemu_extdevice")
+
+static int
+qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt,
+                        virCommandPtr cmd,
+                        const char *info)
+{
+    int ret = -1;
+    char *timestamp = NULL;
+    char *logline = NULL;
+    int logFD;
+
+    logFD = qemuDomainLogContextGetWriteFD(logCtxt);
+
+    if ((timestamp = virTimeStringNow()) == NULL)
+        goto cleanup;
+
+    if (virAsprintf(&logline, "%s: Starting external device: %s\n",
+                    timestamp, info) < 0)
+        goto cleanup;
+
+    if (safewrite(logFD, logline, strlen(logline)) < 0)
+        goto cleanup;
+
+    virCommandWriteArgLog(cmd, logFD);
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(timestamp);
+    VIR_FREE(logline);
+
+    return ret;
+}
+
+
+/*
+ * qemuExtTPMStartEmulator:
+ *
+ * @comm: virConnect pointer
+ * @driver: QEMU driver
+ * @vm: domain object
+ *
+ * Start the external TPM Emulator:
+ * - have the command line built
+ * - start the external TPM Emulator and sync with it before QEMU start
+ */
+static int
+qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
+                        virDomainObjPtr vm,
+                        qemuDomainLogContextPtr logCtxt)
+{
+    int ret = -1;
+    virCommandPtr cmd = NULL;
+    int exitstatus;
+    char *errbuf = NULL;
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    virDomainDefPtr def = vm->def;
+    unsigned char *vmuuid = def->uuid;
+    virDomainTPMDefPtr tpm = def->tpm;
+
+    /* stop any left-over TPM emulator for this VM */
+    virTPMStopEmulator(tpm, vmuuid, false);
+
+    if (!(cmd = virTPMEmulatorBuildCommand(tpm, vmuuid, cfg->swtpm_user)))
+        goto cleanup;
+
+    if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0)
+        goto cleanup;
+
+    virCommandSetErrorBuffer(cmd, &errbuf);
+
+    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
+        VIR_ERROR("Could not start 'swtpm'. exitstatus: %d\n"
+                  "stderr: %s\n", exitstatus, errbuf);
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not start 'swtpm'. exitstatus: %d, "
+                       "error: %s"), exitstatus, errbuf);
+        goto error;
+    }
+
+    /* sync the startup of the swtpm's Unix socket with the start of QEMU */
+    if (virTPMTryConnect(tpm->data.emulator.source.data.nix.path,
+                         3 * 1000 * 1000) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not connect to the swtpm on '%s'"),
+                       tpm->data.emulator.source.data.nix.path);
+        goto error;
+    }
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(errbuf);
+    virCommandFree(cmd);
+
+    virObjectUnref(cfg);
+
+    return ret;
+
+ error:
+    virTPMStopEmulator(tpm, vmuuid, false);
+    VIR_FREE(tpm->data.emulator.source.data.nix.path);
+
+    goto cleanup;
+}
+
+
+static int
+qemuExtTPMStart(virQEMUDriverPtr driver,
+                virDomainObjPtr vm,
+                qemuDomainLogContextPtr logCtxt)
+{
+    int ret = 0;
+    virDomainTPMDefPtr tpm = vm->def->tpm;
+
+    switch (tpm->type) {
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        ret = qemuExtTPMStartEmulator(driver, vm, logCtxt);
+        break;
+    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+    case VIR_DOMAIN_TPM_TYPE_LAST:
+        break;
+    }
+
+    return ret;
+}
+
+static void
+qemuExtTPMStop(virDomainObjPtr vm)
+{
+    switch (vm->def->tpm->type) {
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        virTPMStopEmulator(vm->def->tpm, vm->def->uuid, false);
+        break;
+    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+    case VIR_DOMAIN_TPM_TYPE_LAST:
+        break;
+    }
+}
+
+int
+qemuExtDevicesStart(virQEMUDriverPtr driver,
+                    virDomainObjPtr vm,
+                    qemuDomainLogContextPtr logCtxt)
+{
+    int ret = 0;
+
+    if (vm->def->tpm)
+        ret = qemuExtTPMStart(driver, vm, logCtxt);
+
+    return ret;
+}
+
+void
+qemuExtDevicesStop(virDomainObjPtr vm)
+{
+     if (vm->def->tpm)
+         qemuExtTPMStop(vm);
+}
diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h
new file mode 100644
index 0000000..4dcaec3
--- /dev/null
+++ b/src/qemu/qemu_extdevice.h
@@ -0,0 +1,36 @@
+/*
+ * qemu_extdevice.h: QEMU external devices support
+ *
+ * Copyright (C) 2014 IBM Corporation
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Stefan Berger <stefanb@linux.vnet.ibm.com>
+ */
+#ifndef __QEMU_EXTDEVICE_H__
+# define __QEMU_EXTDEVICE_H__
+
+# include "qemu_conf.h"
+# include "qemu_domain.h"
+
+int qemuExtDevicesStart(virQEMUDriverPtr driver,
+                        virDomainObjPtr vm,
+                        qemuDomainLogContextPtr logCtxt)
+    ATTRIBUTE_RETURN_CHECK;
+
+void qemuExtDevicesStop(virDomainObjPtr vm);
+
+#endif /* __QEMU_EXTDEVICE_H__ */
+
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1afb71f..26acfab 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -47,6 +47,7 @@
 #include "qemu_migration.h"
 #include "qemu_interface.h"
 #include "qemu_security.h"
+#include "qemu_extdevice.h"
 
 #include "cpu/cpu.h"
 #include "datatypes.h"
@@ -5952,6 +5953,9 @@ qemuProcessLaunch(virConnectPtr conn,
         goto cleanup;
     logfile = qemuDomainLogContextGetWriteFD(logCtxt);
 
+    if (qemuExtDevicesStart(driver, vm, logCtxt) < 0)
+        goto cleanup;
+
     VIR_DEBUG("Building emulator command line");
     if (!(cmd = qemuBuildCommandLine(driver,
                                      qemuDomainLogContextGetManager(logCtxt),
@@ -6191,6 +6195,8 @@ qemuProcessLaunch(virConnectPtr conn,
     ret = 0;
 
  cleanup:
+    if (ret)
+        qemuExtDevicesStop(vm);
     qemuDomainSecretDestroy(vm);
     virCommandFree(cmd);
     virObjectUnref(logCtxt);
@@ -6557,6 +6563,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
     /* Clear network bandwidth */
     virDomainClearNetBandwidth(vm);
 
+    qemuExtDevicesStop(vm);
+
     virDomainConfVMNWFilterTeardown(vm);
 
     if (cfg->macFilter) {
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index 688e5b9..03bef74 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -100,3 +100,4 @@ module Test_libvirtd_qemu =
     { "1" = "mount" }
 }
 { "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" }
+{ "swtpm_user" = "tss" }
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 663c8c9..351f6f4 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1372,6 +1372,11 @@ virSecurityDACSetTPMFileLabel(virSecurityManagerPtr mgr,
                                             &tpm->data.passthrough.source,
                                             false);
         break;
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        ret = virSecurityDACSetChardevLabel(mgr, def,
+                                            &tpm->data.emulator.source,
+                                            false);
+        break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
         break;
     }
@@ -1393,6 +1398,7 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr mgr,
                                                 &tpm->data.passthrough.source,
                                                 false);
         break;
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
     case VIR_DOMAIN_TPM_TYPE_LAST:
         break;
     }
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index c26cdac..cfc8311 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1472,6 +1472,12 @@ virSecuritySELinuxSetTPMFileLabel(virSecurityManagerPtr mgr,
             return -1;
         }
         break;
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        tpmdev = tpm->data.emulator.source.data.nix.path;
+        rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, seclabel->imagelabel);
+        if (rc < 0)
+            return -1;
+        break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
         break;
     }
@@ -1505,6 +1511,11 @@ virSecuritySELinuxRestoreTPMFileLabelInt(virSecurityManagerPtr mgr,
             VIR_FREE(cancel_path);
         }
         break;
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        tpmdev = tpm->data.emulator.source.data.nix.path;
+        if (tpmdev)
+            rc = virSecuritySELinuxRestoreFileLabel(mgr, tpmdev);
+        break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
         break;
     }
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 5e9bd20..101f071 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -38,6 +38,7 @@
 #include <unistd.h>
 #include <dirent.h>
 #include <dirname.h>
+#include <ftw.h>
 #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
 # include <mntent.h>
 #endif
@@ -3031,6 +3032,17 @@ virFileMakeParentPath(const char *path)
     return ret;
 }
 
+static int
+_virFileDeletePathCB(const char *fpath, const struct stat *sb ATTRIBUTE_UNUSED,
+                     int typeflag ATTRIBUTE_UNUSED, struct FTW *ftwbuf ATTRIBUTE_UNUSED)
+{
+    return remove(fpath);
+}
+
+int virFileDeletePath(const char *path)
+{
+    return nftw(path, _virFileDeletePathCB, 64, FTW_DEPTH | FTW_PHYS);
+}
 
 /* Build up a fully qualified path for a config file to be
  * associated with a persistent guest or network */
diff --git a/src/util/virfile.h b/src/util/virfile.h
index cd2a386..e7fa736 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -258,6 +258,7 @@ int virFileMakePath(const char *path) ATTRIBUTE_RETURN_CHECK;
 int virFileMakePathWithMode(const char *path,
                             mode_t mode) ATTRIBUTE_RETURN_CHECK;
 int virFileMakeParentPath(const char *path) ATTRIBUTE_RETURN_CHECK;
+int virFileDeletePath(const char *path) ATTRIBUTE_RETURN_CHECK;
 
 char *virFileBuildPath(const char *dir,
                        const char *name,
@@ -353,7 +354,6 @@ int virFileReadValueString(char **value, const char *format, ...)
 
 int virFileWaitForExists(const char *path, size_t ms, size_t tries);
 
-
 int virFileInData(int fd,
                   int *inData,
                   long long *length);
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index d5c10da..8a99876 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -22,16 +22,36 @@
 
 #include <config.h>
 
+#include <sys/types.h>
 #include <sys/stat.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <cap-ng.h>
 
+#include "conf/domain_conf.h"
+#include "viralloc.h"
+#include "vircommand.h"
 #include "virstring.h"
 #include "virerror.h"
 #include "viralloc.h"
 #include "virfile.h"
+#include "virkmod.h"
+#include "virlog.h"
 #include "virtpm.h"
+#include "virutil.h"
+#include "configmake.h"
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
+VIR_LOG_INIT("util.tpm")
+
+/*
+ * executables for the swtpm; to be found on the host
+ */
+static char *swtpm_path;
+static char *swtpm_setup;
+static char *swtpm_ioctl;
+
 /**
  * virTPMCreateCancelPath:
  * @devpath: Path to the TPM device
@@ -74,3 +94,415 @@ virTPMCreateCancelPath(const char *devpath)
  cleanup:
     return path;
 }
+
+/*
+ * virTPMEmulatorInit
+ *
+ * Initialize the Emulator functions by searching for necessary
+ * executables that we will use to start and setup the swtpm
+ */
+static int
+virTPMEmulatorInit(void)
+{
+    if (!swtpm_path) {
+        swtpm_path = virFindFileInPath("swtpm");
+        if (!swtpm_path) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Could not find swtpm 'swtpm' in PATH"));
+            return -1;
+        }
+        if (!virFileIsExecutable(swtpm_path)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("TPM emulator %s is not an executable"),
+                           swtpm_path);
+            VIR_FREE(swtpm_path);
+            return -1;
+        }
+    }
+
+    if (!swtpm_setup) {
+        swtpm_setup = virFindFileInPath("swtpm_setup");
+        if (!swtpm_setup) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Could not find 'swtpm_setup' in PATH"));
+            return -1;
+        }
+        if (!virFileIsExecutable(swtpm_setup)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("'%s' is not an executable"),
+                           swtpm_setup);
+            VIR_FREE(swtpm_setup);
+            return -1;
+        }
+    }
+
+    if (!swtpm_ioctl) {
+        swtpm_ioctl = virFindFileInPath("swtpm_ioctl");
+        if (!swtpm_ioctl) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Could not find swtpm_ioctl in PATH"));
+            return -1;
+        }
+        if (!virFileIsExecutable(swtpm_ioctl)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("swtpm_ioctl program %s is not an executable"),
+                           swtpm_ioctl);
+            VIR_FREE(swtpm_ioctl);
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+/*
+ * virTPMCreateEmulatorStoragePath
+ *
+ * @vmuuid: The UUID of the VM for which to create the storage;
+ *          may be NULL
+ * @suffix: A suffix to append to the storage path; this can be
+ *          used to create a file path
+ *
+ * Create the swtpm's storage path
+ */
+static char *
+virTPMCreateEmulatorStoragePath(const unsigned char *vmuuid,
+                                const char *suffix)
+{
+    char *path = NULL;
+    char uuid[VIR_UUID_STRING_BUFLEN];
+
+    if (vmuuid)
+        virUUIDFormat(vmuuid, uuid);
+    else
+        uuid[0] = '\0';
+
+    if (virAsprintf(&path,
+                    "%s/lib/libvirt/tpm/%s%s",
+                    LOCALSTATEDIR, uuid, suffix) < 0)
+        virReportOOMError();
+
+    return path;
+}
+
+/*
+ * virTPMEmulatorInitStorage
+ *
+ * Initialize the TPM Emulator storage by creating its root directory,
+ * which is typically found in /var/lib/libvirt/tpm.
+ *
+ */
+static int
+virTPMEmulatorInitStorage(void)
+{
+    char *path = NULL;
+    int rc = 0;
+
+    if (!(path = virTPMCreateEmulatorStoragePath(NULL, "")))
+        return -1;
+
+    if (virFileExists(path))
+        goto cleanup;
+
+    /* allow others to cd into this dir */
+    if (virFileMakePathWithMode(path, 0711) < 0) {
+        virReportSystemError(errno,
+                             _("Could not create TPM directory %s"),
+                             path);
+        rc = -1;
+    }
+
+ cleanup:
+    VIR_FREE(path);
+
+    return rc;
+}
+
+/*
+ * virTPMCreateEmulatorStorage
+ *
+ * @vmuuid: The UUID of the VM
+ * @created: a pointer to a bool that will be set to true if the
+ *           storage was created because it did not exist yet
+ * @userid: The userid that needs to be able to access the directory
+ *
+ * Unless the storage path for the swtpm for the given VM
+ * already exists, create it and make it accessible for the given userid.
+ */
+static char *
+virTPMCreateEmulatorStorage(const unsigned char *vmuuid, bool *created,
+                            uid_t swtpm_user)
+{
+    char *path;
+    mode_t mode;
+
+    if (virTPMEmulatorInitStorage() < 0)
+        return NULL;
+
+    *created = false;
+
+    if (!(path = virTPMCreateEmulatorStoragePath(vmuuid, "")))
+        return NULL;
+
+    if (virFileExists(path))
+        goto exit;
+
+    *created = true;
+
+    mode = S_IRUSR | S_IWUSR | S_IXUSR;
+
+    if (virDirCreate(path, mode, swtpm_user, swtpm_user, 0) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not create directory %s as uid %u"),
+                       path, swtpm_user);
+        VIR_FREE(path);
+    }
+
+ exit:
+    return path;
+}
+
+void
+virTPMDeleteEmulatorStorage(const unsigned char *vmuuid)
+{
+    char *path;
+
+    if (!(path = virTPMCreateEmulatorStoragePath(vmuuid, "")))
+        return;
+
+    ignore_value(virFileDeletePath(path));
+    VIR_FREE(path);
+}
+
+
+/*
+ * virTPMCreateEmulatorSocket:
+ *
+ * @uuid: the UUID of the VM
+ *
+ * Create the vTPM device name from the given parameters
+ */
+static char *
+virTPMCreateEmulatorSocket(unsigned const char *vmuuid)
+{
+    return virTPMCreateEmulatorStoragePath(vmuuid, ".sock");
+}
+
+/*
+ * virTPMTryConnect
+ *
+ * @pathname: The device pathname to try to open()
+ * @timeout_ms: The time in ms to spend trying to connect
+ *
+ * Try to connect to the given device pathname using open().
+ */
+int
+virTPMTryConnect(const char *pathname, unsigned long timeout_ms)
+{
+    return virFileWaitForExists(pathname, 10, timeout_ms / 10);
+}
+
+/*
+ * virTPMSetupEmulator
+ *
+ * @storagepath: path to the directory for TPM state
+ * @vmuuid: the UUID of the VM
+ * @userid: The userid to switch to when setting up the TPM;
+ *          typically this should be 'tss'
+ * @logfile: The file to write the log into; it must be writable
+ *           for the user given by userid or 'tss'
+ *
+ * Setup the external swtpm
+ */
+static int
+virTPMSetupEmulator(const char *storagepath, const unsigned char *vmuuid,
+                    uid_t swtpm_user, const char *logfile)
+{
+    virCommandPtr cmd = NULL;
+    int exitstatus;
+    int rc = 0;
+    char uuid[VIR_UUID_STRING_BUFLEN];
+
+    cmd = virCommandNew(swtpm_setup);
+    if (!cmd) {
+        rc = -1;
+        goto cleanup;
+    }
+
+    virUUIDFormat(vmuuid, uuid);
+
+    if (swtpm_user > 0) {
+        virCommandAddArg(cmd, "--runas");
+        virCommandAddArgFormat(cmd, "%u", swtpm_user);
+    }
+    virCommandAddArgList(cmd,
+                         "--tpm-state", storagepath,
+                         "--vmid", uuid,
+                         "--logfile", logfile,
+                         "--createek",
+                         "--create-ek-cert",
+                         "--create-platform-cert",
+                         "--lock-nvram",
+                         "--not-overwrite",
+                         NULL);
+
+    virCommandClearCaps(cmd);
+
+    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
+        /* copy the log to libvirt error since the log will be deleted */
+        char *buffer = NULL;
+        ignore_value(virFileReadAllQuiet(logfile, 10240, &buffer));
+        VIR_ERROR(_("Error setting up swtpm:\n%s"), buffer);
+        VIR_FREE(buffer);
+
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not run '%s'. exitstatus: %d; "
+                         "please check the libvirt error log"),
+                       swtpm_setup, exitstatus);
+        rc = -1;
+    }
+    virCommandFree(cmd);
+
+ cleanup:
+
+    return rc;
+}
+
+/*
+ * virTPMBuildEmulatorCommand
+ *
+ * @tpm: TPM definition
+ * @vmuuid: The UUID of the VM
+ * @swtpm_user: The uid for the swtpm to run as (drop privileges to from root)
+ *
+ * Create the virCommand use for starting the emulator
+ * Do some initializations on the way, such as creation of storage
+ * and emulator setup.
+ */
+virCommandPtr
+virTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, const unsigned char *vmuuid,
+                           uid_t swtpm_user)
+{
+    virCommandPtr cmd = NULL;
+    char *storagepath = NULL;
+    char *logfile = NULL;
+    bool created = false;
+
+    if (virTPMEmulatorInit() < 0)
+        return NULL;
+
+    if (!(storagepath = virTPMCreateEmulatorStorage(vmuuid, &created,
+                                                    swtpm_user)))
+        return NULL;
+
+    /* create logfile in dir where user creating the state will have access */
+    if (!(logfile = virTPMCreateEmulatorStoragePath(vmuuid, "/vtpm.log")))
+        goto error;
+
+    if (created &&
+        virTPMSetupEmulator(storagepath, vmuuid, swtpm_user, logfile) < 0)
+        goto error;
+
+    if (!(tpm->data.emulator.source.data.nix.path =
+          virTPMCreateEmulatorSocket(vmuuid)))
+        goto error;
+
+    unlink(tpm->data.emulator.source.data.nix.path);
+
+    tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+
+    cmd = virCommandNew(swtpm_path);
+    if (!cmd)
+        goto error;
+
+    virCommandClearCaps(cmd);
+
+    virCommandAddArgList(cmd, "socket", "--daemon", "--ctrl", NULL);
+    virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600",
+                           tpm->data.emulator.source.data.nix.path);
+
+    virCommandAddArg(cmd, "--tpmstate");
+    virCommandAddArgFormat(cmd, "dir=%s", storagepath);
+
+    virCommandAddArg(cmd, "--log");
+    virCommandAddArgFormat(cmd, "file=%s", logfile);
+
+    /* allow process to open logfile by root before dropping privileges */
+    virCommandAllowCap(cmd, CAP_DAC_OVERRIDE);
+
+    if (swtpm_user > 0) {
+        virCommandAddArg(cmd, "--runas");
+        virCommandAddArgFormat(cmd, "%u", swtpm_user);
+        virCommandAllowCap(cmd, CAP_SETGID);
+        virCommandAllowCap(cmd, CAP_SETUID);
+    }
+
+    VIR_FREE(storagepath);
+    VIR_FREE(logfile);
+
+    return cmd;
+
+ error:
+    if (created)
+        virTPMDeleteEmulatorStorage(vmuuid);
+
+    VIR_FREE(tpm->data.emulator.source.data.nix.path);
+    VIR_FREE(storagepath);
+    VIR_FREE(logfile);
+
+    virCommandFree(cmd);
+
+    return NULL;
+}
+
+/*
+ * virTPMStopEmulator
+ * @tpm: TPM definition
+ * @vmuuid: the UUID of the VM
+ * @verbose: whether to report errors
+ *
+ * Gracefully stop the swptm
+ */
+void
+virTPMStopEmulator(virDomainTPMDefPtr tpm, const unsigned char *vmuuid,
+                   bool verbose)
+{
+    virCommandPtr cmd;
+    int exitstatus;
+    char *pathname;
+    char *errbuf = NULL;
+
+    (void)vmuuid;
+    if (virTPMEmulatorInit() < 0)
+        return;
+
+    if (!(pathname = virTPMCreateEmulatorSocket(vmuuid)))
+        return;
+
+    cmd = virCommandNew(swtpm_ioctl);
+    if (!cmd) {
+        VIR_FREE(pathname);
+        return;
+    }
+
+    virCommandAddArgList(cmd, "--unix", pathname, "-s", NULL);
+
+    virCommandSetErrorBuffer(cmd, &errbuf);
+
+    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
+        if (verbose)
+            VIR_ERROR(_("Could not run swtpm_ioctl -s '%s'."
+                      " existstatus: %d\nstderr: %s"),
+                      swtpm_ioctl, exitstatus, errbuf);
+    }
+
+    virCommandFree(cmd);
+
+    /* clean up the socket */
+    unlink(pathname);
+    VIR_FREE(pathname);
+
+    VIR_FREE(tpm->data.emulator.source.data.nix.path);
+    tpm->data.emulator.source.type = 0;
+    VIR_FREE(errbuf);
+}
diff --git a/src/util/virtpm.h b/src/util/virtpm.h
index b21fc05..424718b 100644
--- a/src/util/virtpm.h
+++ b/src/util/virtpm.h
@@ -22,6 +22,18 @@
 #ifndef __VIR_TPM_H__
 # define __VIR_TPM_H__
 
+# include "vircommand.h"
+
+typedef struct _virDomainTPMDef virDomainTPMDef;
+typedef virDomainTPMDef *virDomainTPMDefPtr;
+
 char *virTPMCreateCancelPath(const char *devpath) ATTRIBUTE_NOINLINE;
+virCommandPtr virTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
+                          const unsigned char *vmuuid,
+                          uid_t swtpm_user) ATTRIBUTE_RETURN_CHECK;
+void virTPMStopEmulator(virDomainTPMDefPtr tpm, const unsigned char *vmuuid,
+                       bool verbose);
+void virTPMDeleteEmulatorStorage(const unsigned char *vmuuid);
+int virTPMTryConnect(const char *pathname, unsigned long timeout_ms);
 
 #endif /* __VIR_TPM_H__ */
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index 70a35ef..376f58a 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -150,6 +150,7 @@
   <flag name='virtio-keyboard-ccw'/>
   <flag name='virtio-mouse-ccw'/>
   <flag name='virtio-tablet-ccw'/>
+  <flag name='tpm-emulator'/>
   <version>2011000</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>342058</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
index ff48293..069e0ae 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
@@ -187,6 +187,7 @@
   <flag name='isa-serial'/>
   <flag name='pl011'/>
   <flag name='dump-completed'/>
+  <flag name='tpm-emulator'/>
   <version>2011090</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>342346</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index ee7fb9e..46d2463 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -185,6 +185,7 @@
   <flag name='isa-serial'/>
   <flag name='machine.pseries.max-cpu-compat'/>
   <flag name='dump-completed'/>
+  <flag name='tpm-emulator'/>
   <version>2011090</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>419215</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
index b5b6b5b..36ffd75 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
@@ -150,6 +150,7 @@
   <flag name='virtio-keyboard-ccw'/>
   <flag name='virtio-mouse-ccw'/>
   <flag name='virtio-tablet-ccw'/>
+  <flag name='tpm-emulator'/>
   <version>2011090</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>0</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
index 39ee4f4..b2f06b3 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
@@ -226,6 +226,7 @@
   <flag name='isa-serial'/>
   <flag name='dump-completed'/>
   <flag name='tpm-crb'/>
+  <flag name='tpm-emulator'/>
   <version>2011090</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>390060</microcodeVersion>
diff --git a/tests/qemuxml2argvdata/tpm-emulator.args b/tests/qemuxml2argvdata/tpm-emulator.args
new file mode 100644
index 0000000..9418c74
--- /dev/null
+++ b/tests/qemuxml2argvdata/tpm-emulator.args
@@ -0,0 +1,24 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name TPM-VM \
+-S \
+-M pc-0.12 \
+-m 2048 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 11d7cd22-da89-3094-6212-079a48a309a1 \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-TPM-VM/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-boot c \
+-usb \
+-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \
+-chardev socket,id=chrtpm,path=/dev/test \
+-device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/tpm-emulator.xml b/tests/qemuxml2argvdata/tpm-emulator.xml
new file mode 100644
index 0000000..2f4e777
--- /dev/null
+++ b/tests/qemuxml2argvdata/tpm-emulator.xml
@@ -0,0 +1,30 @@
+<domain type='qemu'>
+  <name>TPM-VM</name>
+  <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>512288</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-0.12'>hvm</type>
+    <boot dev='hd'/>
+    <bootmenu enable='yes'/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <tpm model='tpm-tis'>
+      <backend type='emulator'/>
+    </tpm>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 177b24e..2b970fe 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -161,6 +161,7 @@ virNetDevRunEthernetScript(const char *ifname ATTRIBUTE_UNUSED,
     return 0;
 }
 
+#if 0
 void
 virCommandPassFD(virCommandPtr cmd ATTRIBUTE_UNUSED,
                  int fd ATTRIBUTE_UNUSED,
@@ -168,6 +169,7 @@ virCommandPassFD(virCommandPtr cmd ATTRIBUTE_UNUSED,
 {
     /* nada */
 }
+#endif
 
 uint8_t *
 virCryptoGenerateRandom(size_t nbytes)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 2992197..92846c3 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -497,6 +497,19 @@ testCompareXMLToArgv(const void *data)
         }
     }
 
+    if (vm->def->tpm) {
+       switch (vm->def->tpm->type) {
+       case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+           if (VIR_STRDUP(vm->def->tpm->data.emulator.source.data.file.path,
+                          "/dev/test") < 0)
+               goto cleanup;
+           break;
+       case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+       case VIR_DOMAIN_TPM_TYPE_LAST:
+           break;
+       }
+    }
+
     if (!(cmd = qemuProcessCreatePretendCmd(&driver, vm, migrateURI,
                                             (flags & FLAG_FIPS), false,
                                             VIR_QEMU_PROCESS_START_COLD))) {
@@ -504,6 +517,7 @@ testCompareXMLToArgv(const void *data)
             goto ok;
         goto cleanup;
     }
+
     if (flags & FLAG_EXPECT_FAILURE) {
         VIR_TEST_DEBUG("passed instead of expected failure");
         goto cleanup;
@@ -2139,6 +2153,9 @@ mymain(void)
             QEMU_CAPS_DEVICE_TPM_CRB);
     DO_TEST_PARSE_ERROR("tpm-no-backend-invalid",
                         QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS);
+    DO_TEST("tpm-emulator",
+            QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_EMULATOR,
+            QEMU_CAPS_DEVICE_TPM_TIS);
 
 
     DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE);
diff --git a/tests/qemuxml2xmloutdata/tpm-emulator.xml b/tests/qemuxml2xmloutdata/tpm-emulator.xml
new file mode 100644
index 0000000..1f783bb
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/tpm-emulator.xml
@@ -0,0 +1,34 @@
+<domain type='qemu'>
+  <name>TPM-VM</name>
+  <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>512288</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-0.12'>hvm</type>
+    <boot dev='hd'/>
+    <bootmenu enable='yes'/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <tpm model='tpm-tis'>
+      <backend type='emulator'/>
+    </tpm>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0f56029..b3e7c8e 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -700,6 +700,7 @@ mymain(void)
     DO_TEST("usb-ich9-ehci-addr", NONE);
     DO_TEST("disk-copy_on_read", NONE);
     DO_TEST("tpm-passthrough", NONE);
+    DO_TEST("tpm-emulator", NONE);
 
     DO_TEST("metadata", NONE);
     DO_TEST("metadata-duplicate", NONE);
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] tpm: Add support for external swtpm TPM emulator
Posted by Daniel P. Berrangé 7 years, 1 month ago
On Thu, Apr 05, 2018 at 05:56:02PM -0400, Stefan Berger wrote:
> This patch adds support for an external swtpm TPM emulator. The XML for
> this type of TPM looks as follows:
> 
>  <tpm model='tpm-tis'>
>    <backend type='emulator'/>
>  </tpm>
> 
> The XML will currently only start a TPM 1.2.
> 
> Upon the first start, libvirt will run `swtpm_setup`, which will simulate the
> manufacturing of a TPM and create certificates for it and write them into the
> NVRAM location of the emulated TPM.
> 
> Then, libvirt will automatically start the swtpm TPM emulator using the `swtpm`
> executable.
> 
> Once the VM terminates, libvirt uses the swtpm_ioctl executable to gracefully
> shut down the `swtpm` in case it is still running (QEMU did not send shutdown)
> or clean up the socket file.
> 
> The above mentioned executables must be found in the PATH.
> 
> The executables can either be run as root or started as root and switch to
> the tss user. The requirement for the tss user comes through 'tcsd', which
> is used for the simulation of the manufacturing. Which user is used can be
> configured through qemu.conf.
> 
> The swtpm writes out state into files. The state is kept in /var/lib/libvirt/tpm:
> 
> [root@localhost libvirt]# ls -lZ | grep tpm
> 
> drwx--x--x. 7 root root unconfined_u:object_r:virt_var_lib_t:s0 4096 Apr  5 16:22 tpm
> 
> The directory /var/lib/libvirt/tpm maintains per-TPM state directories but
> also hosts the UnixIO socket of running swtpms, which QEMU uses for communicating
> with them. At this point only the socket file is labeled properly and made accessible
> for QEMU, which runs under the qemu user:

/var/lib is for persistent state while /var/run is for transient
state, so I think sockets should be under /var/run instead.

> 
> [root@localhost tpm]# ls -lZ
> total 4
> drwx------. 2 tss  tss  system_u:object_r:virt_var_lib_t:s0          4096 Apr  5 16:46 485d0004-a48f-436a-8457-8a3b73e28567
> srw-------. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c413,c430    0 Apr  5 16:46 485d0004-a48f-436a-8457-8a3b73e28567.sock
> 
> [root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ls -lZ
> total 8
> -rw-r--r--. 1 tss tss system_u:object_r:virt_var_lib_t:s0 3648 Apr  5 16:46 tpm-00.permall
> -rw-r--r--. 1 tss tss system_u:object_r:virt_var_lib_t:s0 2237 Apr  5 16:46 vtpm.log
> 
> root@sbct-3 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep -v grep
> system_u:system_r:virtd_t:s0-s0:c0.c1023 tss 18697 0.0  0.0 28172 3892 ?       Ss   16:46   0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567.sock,mode=0600 --tpmstate dir=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567 --log file=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567/vtpm.log --runas 59
> 
> [root@sbct-3 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep
> system_u:system_r:svirt_t:s0:c413,c430 qemu 18702 2.5  0.0 3036052 48676 ?     Sl   16:46   0:08 /bin/qemu-system-x86_64 -name guest=centos7.0,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-6-centos7.0/master-key.aes -machine pc-i440fx-2.8,accel=kvm,usb=off,dump-guest-core=off -cpu kvm64 -m 2048 -realtime mlock=off -smp 2,sockets=2,cores=1,threads=1 -uuid 485d0004-a48f-436a-8457-8a3b73e28567 [...] -tpmdev emulator,id=tpm-tpm0,chardev=chrtpm -chardev socket,id=chrtpm,path=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567.sock -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 -device usb-mouse,id=input0,bus=usb.0,port=1 -vnc 127.0.0.1:0 -device cirrus-vga,id=video0,bus=pci.0,addr=0x2 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 -msg timestamp=on
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  docs/formatdomain.html.in                          |  30 ++
>  docs/schemas/domaincommon.rng                      |   5 +
>  src/conf/domain_audit.c                            |   2 +
>  src/conf/domain_conf.c                             |  51 ++-
>  src/conf/domain_conf.h                             |   5 +
>  src/libvirt_private.syms                           |   5 +
>  src/qemu/Makefile.inc.am                           |   2 +
>  src/qemu/libvirtd_qemu.aug                         |   3 +
>  src/qemu/qemu.conf                                 |   7 +
>  src/qemu/qemu_capabilities.c                       |   5 +
>  src/qemu/qemu_capabilities.h                       |   1 +
>  src/qemu/qemu_cgroup.c                             |   1 +
>  src/qemu/qemu_command.c                            |  52 ++-
>  src/qemu/qemu_conf.c                               |  11 +-
>  src/qemu/qemu_conf.h                               |   2 +
>  src/qemu/qemu_domain.c                             |   2 +
>  src/qemu/qemu_driver.c                             |  13 +
>  src/qemu/qemu_extdevice.c                          | 195 ++++++++++
>  src/qemu/qemu_extdevice.h                          |  36 ++
>  src/qemu/qemu_process.c                            |   8 +
>  src/qemu/test_libvirtd_qemu.aug.in                 |   1 +
>  src/security/security_dac.c                        |   6 +
>  src/security/security_selinux.c                    |  11 +
>  src/util/virfile.c                                 |  12 +
>  src/util/virfile.h                                 |   2 +-
>  src/util/virtpm.c                                  | 432 +++++++++++++++++++++
>  src/util/virtpm.h                                  |  12 +
>  tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   |   1 +
>  tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml |   1 +
>  tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |   1 +
>  tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   |   1 +
>  tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   1 +
>  tests/qemuxml2argvdata/tpm-emulator.args           |  24 ++
>  tests/qemuxml2argvdata/tpm-emulator.xml            |  30 ++
>  tests/qemuxml2argvmock.c                           |   2 +
>  tests/qemuxml2argvtest.c                           |  17 +
>  tests/qemuxml2xmloutdata/tpm-emulator.xml          |  34 ++
>  tests/qemuxml2xmltest.c                            |   1 +
>  38 files changed, 1011 insertions(+), 14 deletions(-)
>  create mode 100644 src/qemu/qemu_extdevice.c
>  create mode 100644 src/qemu/qemu_extdevice.h
>  create mode 100644 tests/qemuxml2argvdata/tpm-emulator.args
>  create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml
>  create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml

> +/*
> + * qemuExtTPMStartEmulator:
> + *
> + * @comm: virConnect pointer
> + * @driver: QEMU driver
> + * @vm: domain object
> + *
> + * Start the external TPM Emulator:
> + * - have the command line built
> + * - start the external TPM Emulator and sync with it before QEMU start
> + */
> +static int
> +qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
> +                        virDomainObjPtr vm,
> +                        qemuDomainLogContextPtr logCtxt)
> +{
> +    int ret = -1;
> +    virCommandPtr cmd = NULL;
> +    int exitstatus;
> +    char *errbuf = NULL;
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    virDomainDefPtr def = vm->def;
> +    unsigned char *vmuuid = def->uuid;
> +    virDomainTPMDefPtr tpm = def->tpm;
> +
> +    /* stop any left-over TPM emulator for this VM */
> +    virTPMStopEmulator(tpm, vmuuid, false);
> +
> +    if (!(cmd = virTPMEmulatorBuildCommand(tpm, vmuuid, cfg->swtpm_user)))
> +        goto cleanup;
> +
> +    if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0)
> +        goto cleanup;
> +
> +    virCommandSetErrorBuffer(cmd, &errbuf);
> +
> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> +        VIR_ERROR("Could not start 'swtpm'. exitstatus: %d\n"
> +                  "stderr: %s\n", exitstatus, errbuf);
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Could not start 'swtpm'. exitstatus: %d, "
> +                       "error: %s"), exitstatus, errbuf);
> +        goto error;
> +    }
> +
> +    /* sync the startup of the swtpm's Unix socket with the start of QEMU */
> +    if (virTPMTryConnect(tpm->data.emulator.source.data.nix.path,
> +                         3 * 1000 * 1000) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Could not connect to the swtpm on '%s'"),
> +                       tpm->data.emulator.source.data.nix.path);
> +        goto error;
> +    }

Ewww, this is really not nice to deal with startup races.

You're using the --daemon flag to swtpm, so swtpm should make sure
that it doesn't daemonize itself & let the parent exit, until the
UNIX socket is actually ready to access connections. Can we just
get swtpm fixed in this way.


> +/*
> + * virTPMSetupEmulator
> + *
> + * @storagepath: path to the directory for TPM state
> + * @vmuuid: the UUID of the VM
> + * @userid: The userid to switch to when setting up the TPM;
> + *          typically this should be 'tss'
> + * @logfile: The file to write the log into; it must be writable
> + *           for the user given by userid or 'tss'
> + *
> + * Setup the external swtpm
> + */
> +static int
> +virTPMSetupEmulator(const char *storagepath, const unsigned char *vmuuid,
> +                    uid_t swtpm_user, const char *logfile)
> +{
> +    virCommandPtr cmd = NULL;
> +    int exitstatus;
> +    int rc = 0;
> +    char uuid[VIR_UUID_STRING_BUFLEN];
> +
> +    cmd = virCommandNew(swtpm_setup);
> +    if (!cmd) {
> +        rc = -1;
> +        goto cleanup;
> +    }
> +
> +    virUUIDFormat(vmuuid, uuid);
> +
> +    if (swtpm_user > 0) {
> +        virCommandAddArg(cmd, "--runas");
> +        virCommandAddArgFormat(cmd, "%u", swtpm_user);
> +    }
> +    virCommandAddArgList(cmd,
> +                         "--tpm-state", storagepath,
> +                         "--vmid", uuid,
> +                         "--logfile", logfile,
> +                         "--createek",
> +                         "--create-ek-cert",
> +                         "--create-platform-cert",
> +                         "--lock-nvram",
> +                         "--not-overwrite",
> +                         NULL);
> +
> +    virCommandClearCaps(cmd);
> +
> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> +        /* copy the log to libvirt error since the log will be deleted */
> +        char *buffer = NULL;
> +        ignore_value(virFileReadAllQuiet(logfile, 10240, &buffer));
> +        VIR_ERROR(_("Error setting up swtpm:\n%s"), buffer);

This is not nice - VIR_ERROR should never be used to report problems
that occurr during guest startup - it needs to be in the error message
reported...

> +        VIR_FREE(buffer);
> +
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Could not run '%s'. exitstatus: %d; "
> +                         "please check the libvirt error log"),
> +                       swtpm_setup, exitstatus);

....here.


> +    cmd = virCommandNew(swtpm_path);
> +    if (!cmd)
> +        goto error;
> +
> +    virCommandClearCaps(cmd);
> +
> +    virCommandAddArgList(cmd, "socket", "--daemon", "--ctrl", NULL);
> +    virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600",
> +                           tpm->data.emulator.source.data.nix.path);
> +
> +    virCommandAddArg(cmd, "--tpmstate");
> +    virCommandAddArgFormat(cmd, "dir=%s", storagepath);
> +
> +    virCommandAddArg(cmd, "--log");
> +    virCommandAddArgFormat(cmd, "file=%s", logfile);
> +
> +    /* allow process to open logfile by root before dropping privileges */
> +    virCommandAllowCap(cmd, CAP_DAC_OVERRIDE);

Why can't we get have the log file be owned by the user that
swtpm runs as, instead of root ?

> +    if (swtpm_user > 0) {
> +        virCommandAddArg(cmd, "--runas");
> +        virCommandAddArgFormat(cmd, "%u", swtpm_user);
> +        virCommandAllowCap(cmd, CAP_SETGID);
> +        virCommandAllowCap(cmd, CAP_SETUID);
> +    }

Then we could tell virCommand to set the UID/GID before it even
executes this, and not use -runas, thus avoiding the need to
allow theses capabilities.


> +/*
> + * virTPMStopEmulator
> + * @tpm: TPM definition
> + * @vmuuid: the UUID of the VM
> + * @verbose: whether to report errors
> + *
> + * Gracefully stop the swptm
> + */
> +void
> +virTPMStopEmulator(virDomainTPMDefPtr tpm, const unsigned char *vmuuid,
> +                   bool verbose)
> +{
> +    virCommandPtr cmd;
> +    int exitstatus;
> +    char *pathname;
> +    char *errbuf = NULL;
> +
> +    (void)vmuuid;
> +    if (virTPMEmulatorInit() < 0)
> +        return;
> +
> +    if (!(pathname = virTPMCreateEmulatorSocket(vmuuid)))
> +        return;
> +
> +    cmd = virCommandNew(swtpm_ioctl);
> +    if (!cmd) {
> +        VIR_FREE(pathname);
> +        return;
> +    }
> +
> +    virCommandAddArgList(cmd, "--unix", pathname, "-s", NULL);
> +
> +    virCommandSetErrorBuffer(cmd, &errbuf);
> +
> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> +        if (verbose)
> +            VIR_ERROR(_("Could not run swtpm_ioctl -s '%s'."
> +                      " existstatus: %d\nstderr: %s"),
> +                      swtpm_ioctl, exitstatus, errbuf);
> +    }
> +
> +    virCommandFree(cmd);

I would feel better if we just directly killed the process - with
this approach if something goes wrong with swtpm it may never
respond to this request and stay running.

If we're starting one swtpm per QEMU, we should also make sure it gets
put into the cgroup associated with that QEMU

> +
> +    /* clean up the socket */
> +    unlink(pathname);
> +    VIR_FREE(pathname);
> +
> +    VIR_FREE(tpm->data.emulator.source.data.nix.path);
> +    tpm->data.emulator.source.type = 0;
> +    VIR_FREE(errbuf);
> +}

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] tpm: Add support for external swtpm TPM emulator
Posted by Stefan Berger 7 years, 1 month ago
On 04/06/2018 04:26 AM, Daniel P. Berrangé wrote:
> On Thu, Apr 05, 2018 at 05:56:02PM -0400, Stefan Berger wrote:
>> This patch adds support for an external swtpm TPM emulator. The XML for
>> this type of TPM looks as follows:
>>
>>   <tpm model='tpm-tis'>
>>     <backend type='emulator'/>
>>   </tpm>
>>
>> The XML will currently only start a TPM 1.2.
>>
>> Upon the first start, libvirt will run `swtpm_setup`, which will simulate the
>> manufacturing of a TPM and create certificates for it and write them into the
>> NVRAM location of the emulated TPM.
>>
>> Then, libvirt will automatically start the swtpm TPM emulator using the `swtpm`
>> executable.
>>
>> Once the VM terminates, libvirt uses the swtpm_ioctl executable to gracefully
>> shut down the `swtpm` in case it is still running (QEMU did not send shutdown)
>> or clean up the socket file.
>>
>> The above mentioned executables must be found in the PATH.
>>
>> The executables can either be run as root or started as root and switch to
>> the tss user. The requirement for the tss user comes through 'tcsd', which
>> is used for the simulation of the manufacturing. Which user is used can be
>> configured through qemu.conf.
>>
>> The swtpm writes out state into files. The state is kept in /var/lib/libvirt/tpm:
>>
>> [root@localhost libvirt]# ls -lZ | grep tpm
>>
>> drwx--x--x. 7 root root unconfined_u:object_r:virt_var_lib_t:s0 4096 Apr  5 16:22 tpm
>>
>> The directory /var/lib/libvirt/tpm maintains per-TPM state directories but
>> also hosts the UnixIO socket of running swtpms, which QEMU uses for communicating
>> with them. At this point only the socket file is labeled properly and made accessible
>> for QEMU, which runs under the qemu user:
> /var/lib is for persistent state while /var/run is for transient
> state, so I think sockets should be under /var/run instead.

/var/run/libvirt/qemu then ?


>
>> [root@localhost tpm]# ls -lZ
>> total 4
>> drwx------. 2 tss  tss  system_u:object_r:virt_var_lib_t:s0          4096 Apr  5 16:46 485d0004-a48f-436a-8457-8a3b73e28567
>> srw-------. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c413,c430    0 Apr  5 16:46 485d0004-a48f-436a-8457-8a3b73e28567.sock
>>
>> [root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ls -lZ
>> total 8
>> -rw-r--r--. 1 tss tss system_u:object_r:virt_var_lib_t:s0 3648 Apr  5 16:46 tpm-00.permall
>> -rw-r--r--. 1 tss tss system_u:object_r:virt_var_lib_t:s0 2237 Apr  5 16:46 vtpm.log
>>
>> root@sbct-3 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep -v grep
>> system_u:system_r:virtd_t:s0-s0:c0.c1023 tss 18697 0.0  0.0 28172 3892 ?       Ss   16:46   0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567.sock,mode=0600 --tpmstate dir=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567 --log file=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567/vtpm.log --runas 59
>>
>> [root@sbct-3 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep
>> system_u:system_r:svirt_t:s0:c413,c430 qemu 18702 2.5  0.0 3036052 48676 ?     Sl   16:46   0:08 /bin/qemu-system-x86_64 -name guest=centos7.0,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-6-centos7.0/master-key.aes -machine pc-i440fx-2.8,accel=kvm,usb=off,dump-guest-core=off -cpu kvm64 -m 2048 -realtime mlock=off -smp 2,sockets=2,cores=1,threads=1 -uuid 485d0004-a48f-436a-8457-8a3b73e28567 [...] -tpmdev emulator,id=tpm-tpm0,chardev=chrtpm -chardev socket,id=chrtpm,path=/var/lib/libvirt/tpm/485d0004-a48f-436a-8457-8a3b73e28567.sock -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 -device usb-mouse,id=input0,bus=usb.0,port=1 -vnc 127.0.0.1:0 -device cirrus-vga,id=video0,bus=pci.0,addr=0x2 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 -msg timestamp=on
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>>   docs/formatdomain.html.in                          |  30 ++
>>   docs/schemas/domaincommon.rng                      |   5 +
>>   src/conf/domain_audit.c                            |   2 +
>>   src/conf/domain_conf.c                             |  51 ++-
>>   src/conf/domain_conf.h                             |   5 +
>>   src/libvirt_private.syms                           |   5 +
>>   src/qemu/Makefile.inc.am                           |   2 +
>>   src/qemu/libvirtd_qemu.aug                         |   3 +
>>   src/qemu/qemu.conf                                 |   7 +
>>   src/qemu/qemu_capabilities.c                       |   5 +
>>   src/qemu/qemu_capabilities.h                       |   1 +
>>   src/qemu/qemu_cgroup.c                             |   1 +
>>   src/qemu/qemu_command.c                            |  52 ++-
>>   src/qemu/qemu_conf.c                               |  11 +-
>>   src/qemu/qemu_conf.h                               |   2 +
>>   src/qemu/qemu_domain.c                             |   2 +
>>   src/qemu/qemu_driver.c                             |  13 +
>>   src/qemu/qemu_extdevice.c                          | 195 ++++++++++
>>   src/qemu/qemu_extdevice.h                          |  36 ++
>>   src/qemu/qemu_process.c                            |   8 +
>>   src/qemu/test_libvirtd_qemu.aug.in                 |   1 +
>>   src/security/security_dac.c                        |   6 +
>>   src/security/security_selinux.c                    |  11 +
>>   src/util/virfile.c                                 |  12 +
>>   src/util/virfile.h                                 |   2 +-
>>   src/util/virtpm.c                                  | 432 +++++++++++++++++++++
>>   src/util/virtpm.h                                  |  12 +
>>   tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   |   1 +
>>   tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml |   1 +
>>   tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |   1 +
>>   tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   |   1 +
>>   tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   1 +
>>   tests/qemuxml2argvdata/tpm-emulator.args           |  24 ++
>>   tests/qemuxml2argvdata/tpm-emulator.xml            |  30 ++
>>   tests/qemuxml2argvmock.c                           |   2 +
>>   tests/qemuxml2argvtest.c                           |  17 +
>>   tests/qemuxml2xmloutdata/tpm-emulator.xml          |  34 ++
>>   tests/qemuxml2xmltest.c                            |   1 +
>>   38 files changed, 1011 insertions(+), 14 deletions(-)
>>   create mode 100644 src/qemu/qemu_extdevice.c
>>   create mode 100644 src/qemu/qemu_extdevice.h
>>   create mode 100644 tests/qemuxml2argvdata/tpm-emulator.args
>>   create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml
>>   create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml
>> +/*
>> + * qemuExtTPMStartEmulator:
>> + *
>> + * @comm: virConnect pointer
>> + * @driver: QEMU driver
>> + * @vm: domain object
>> + *
>> + * Start the external TPM Emulator:
>> + * - have the command line built
>> + * - start the external TPM Emulator and sync with it before QEMU start
>> + */
>> +static int
>> +qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
>> +                        virDomainObjPtr vm,
>> +                        qemuDomainLogContextPtr logCtxt)
>> +{
>> +    int ret = -1;
>> +    virCommandPtr cmd = NULL;
>> +    int exitstatus;
>> +    char *errbuf = NULL;
>> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>> +    virDomainDefPtr def = vm->def;
>> +    unsigned char *vmuuid = def->uuid;
>> +    virDomainTPMDefPtr tpm = def->tpm;
>> +
>> +    /* stop any left-over TPM emulator for this VM */
>> +    virTPMStopEmulator(tpm, vmuuid, false);
>> +
>> +    if (!(cmd = virTPMEmulatorBuildCommand(tpm, vmuuid, cfg->swtpm_user)))
>> +        goto cleanup;
>> +
>> +    if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0)
>> +        goto cleanup;
>> +
>> +    virCommandSetErrorBuffer(cmd, &errbuf);
>> +
>> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
>> +        VIR_ERROR("Could not start 'swtpm'. exitstatus: %d\n"
>> +                  "stderr: %s\n", exitstatus, errbuf);
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("Could not start 'swtpm'. exitstatus: %d, "
>> +                       "error: %s"), exitstatus, errbuf);
>> +        goto error;
>> +    }
>> +
>> +    /* sync the startup of the swtpm's Unix socket with the start of QEMU */
>> +    if (virTPMTryConnect(tpm->data.emulator.source.data.nix.path,
>> +                         3 * 1000 * 1000) < 0) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("Could not connect to the swtpm on '%s'"),
>> +                       tpm->data.emulator.source.data.nix.path);
>> +        goto error;
>> +    }
> Ewww, this is really not nice to deal with startup races.
>
> You're using the --daemon flag to swtpm, so swtpm should make sure
> that it doesn't daemonize itself & let the parent exit, until the
> UNIX socket is actually ready to access connections. Can we just
> get swtpm fixed in this way.

It does that actually already and the socket file is there once it 
daemonizes itself.

>
>
>> +/*
>> + * virTPMSetupEmulator
>> + *
>> + * @storagepath: path to the directory for TPM state
>> + * @vmuuid: the UUID of the VM
>> + * @userid: The userid to switch to when setting up the TPM;
>> + *          typically this should be 'tss'
>> + * @logfile: The file to write the log into; it must be writable
>> + *           for the user given by userid or 'tss'
>> + *
>> + * Setup the external swtpm
>> + */
>> +static int
>> +virTPMSetupEmulator(const char *storagepath, const unsigned char *vmuuid,
>> +                    uid_t swtpm_user, const char *logfile)
>> +{
>> +    virCommandPtr cmd = NULL;
>> +    int exitstatus;
>> +    int rc = 0;
>> +    char uuid[VIR_UUID_STRING_BUFLEN];
>> +
>> +    cmd = virCommandNew(swtpm_setup);
>> +    if (!cmd) {
>> +        rc = -1;
>> +        goto cleanup;
>> +    }
>> +
>> +    virUUIDFormat(vmuuid, uuid);
>> +
>> +    if (swtpm_user > 0) {
>> +        virCommandAddArg(cmd, "--runas");
>> +        virCommandAddArgFormat(cmd, "%u", swtpm_user);
>> +    }
>> +    virCommandAddArgList(cmd,
>> +                         "--tpm-state", storagepath,
>> +                         "--vmid", uuid,
>> +                         "--logfile", logfile,
>> +                         "--createek",
>> +                         "--create-ek-cert",
>> +                         "--create-platform-cert",
>> +                         "--lock-nvram",
>> +                         "--not-overwrite",
>> +                         NULL);
>> +
>> +    virCommandClearCaps(cmd);
>> +
>> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
>> +        /* copy the log to libvirt error since the log will be deleted */
>> +        char *buffer = NULL;
>> +        ignore_value(virFileReadAllQuiet(logfile, 10240, &buffer));
>> +        VIR_ERROR(_("Error setting up swtpm:\n%s"), buffer);
> This is not nice - VIR_ERROR should never be used to report problems
> that occurr during guest startup - it needs to be in the error message
> reported...
>
>> +        VIR_FREE(buffer);
>> +
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("Could not run '%s'. exitstatus: %d; "
>> +                         "please check the libvirt error log"),
>> +                       swtpm_setup, exitstatus);
> ....here.

Ok. will display it here.

>
>
>> +    cmd = virCommandNew(swtpm_path);
>> +    if (!cmd)
>> +        goto error;
>> +
>> +    virCommandClearCaps(cmd);
>> +
>> +    virCommandAddArgList(cmd, "socket", "--daemon", "--ctrl", NULL);
>> +    virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600",
>> +                           tpm->data.emulator.source.data.nix.path);
>> +
>> +    virCommandAddArg(cmd, "--tpmstate");
>> +    virCommandAddArgFormat(cmd, "dir=%s", storagepath);
>> +
>> +    virCommandAddArg(cmd, "--log");
>> +    virCommandAddArgFormat(cmd, "file=%s", logfile);
>> +
>> +    /* allow process to open logfile by root before dropping privileges */
>> +    virCommandAllowCap(cmd, CAP_DAC_OVERRIDE);
> Why can't we get have the log file be owned by the user that
> swtpm runs as, instead of root ?

I would have to look at this particular capability again. I initially 
wanted to put the swtpm's log file also into /var/log/libvirt/qemu. It 
works nice of course when running swtpm as 'root' but not so much when 
running it as 'tss':

root@localhost tmp]$ sudo ls -l /var/log/libvirt/ | grep qemu
drwx------. 2 root root 20480 Apr  5 16:14 qemu

So where do we put the swtpm's log files? /var/log/libvirt/swtpm?

Iirc the CAP_DAC_OVERRIDE became necessary when swtpm tries to append to 
the log that 'tss' owns but now libvirt runs it as 'root'. I think that 
was the reason I added it. One way to solve this would be to chown() the 
files before starting swtpm. Is that the solution?


>
>> +    if (swtpm_user > 0) {
>> +        virCommandAddArg(cmd, "--runas");
>> +        virCommandAddArgFormat(cmd, "%u", swtpm_user);
>> +        virCommandAllowCap(cmd, CAP_SETGID);
>> +        virCommandAllowCap(cmd, CAP_SETUID);
>> +    }
> Then we could tell virCommand to set the UID/GID before it even
> executes this, and not use -runas, thus avoiding the need to
> allow theses capabilities.

And the directory it writes the logs in would have to have ownership of 
either root:root or tss:root (or tss:tss), depending on how libvirt is 
currently configured? Again chown() the dir before starting?

>
>
>> +/*
>> + * virTPMStopEmulator
>> + * @tpm: TPM definition
>> + * @vmuuid: the UUID of the VM
>> + * @verbose: whether to report errors
>> + *
>> + * Gracefully stop the swptm
>> + */
>> +void
>> +virTPMStopEmulator(virDomainTPMDefPtr tpm, const unsigned char *vmuuid,
>> +                   bool verbose)
>> +{
>> +    virCommandPtr cmd;
>> +    int exitstatus;
>> +    char *pathname;
>> +    char *errbuf = NULL;
>> +
>> +    (void)vmuuid;
>> +    if (virTPMEmulatorInit() < 0)
>> +        return;
>> +
>> +    if (!(pathname = virTPMCreateEmulatorSocket(vmuuid)))
>> +        return;
>> +
>> +    cmd = virCommandNew(swtpm_ioctl);
>> +    if (!cmd) {
>> +        VIR_FREE(pathname);
>> +        return;
>> +    }
>> +
>> +    virCommandAddArgList(cmd, "--unix", pathname, "-s", NULL);
>> +
>> +    virCommandSetErrorBuffer(cmd, &errbuf);
>> +
>> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
>> +        if (verbose)
>> +            VIR_ERROR(_("Could not run swtpm_ioctl -s '%s'."
>> +                      " existstatus: %d\nstderr: %s"),
>> +                      swtpm_ioctl, exitstatus, errbuf);
>> +    }
>> +
>> +    virCommandFree(cmd);
> I would feel better if we just directly killed the process - with
> this approach if something goes wrong with swtpm it may never
> respond to this request and stay running.

swtpm can write a pidfile. I am only adding this later in this series. 
Problem is with --daemon libvirt doesn't know the pid of the swtpm anymore.

>
> If we're starting one swtpm per QEMU, we should also make sure it gets
> put into the cgroup associated with that QEMU

That's done in patch 6/6.

>
>> +
>> +    /* clean up the socket */
>> +    unlink(pathname);
>> +    VIR_FREE(pathname);
>> +
>> +    VIR_FREE(tpm->data.emulator.source.data.nix.path);
>> +    tpm->data.emulator.source.type = 0;
>> +    VIR_FREE(errbuf);
>> +}
> Regards,
> Daniel

Thanks a lot!

    Stefan


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] tpm: Add support for external swtpm TPM emulator
Posted by Stefan Berger 7 years, 1 month ago
On 04/06/2018 07:23 AM, Stefan Berger wrote:
> On 04/06/2018 04:26 AM, Daniel P. Berrangé wrote:
>> On Thu, Apr 05, 2018 at 05:56:02PM -0400, Stefan Berger wrote:
>>> This patch adds support for an external swtpm TPM emulator. The XML for
>>> this type of TPM looks as follows:
>>>
>>>   <tpm model='tpm-tis'>
>>>     <backend type='emulator'/>
>>>   </tpm>
>>>
>>> The XML will currently only start a TPM 1.2.
>>>
>>> Upon the first start, libvirt will run `swtpm_setup`, which will 
>>> simulate the
>>> manufacturing of a TPM and create certificates for it and write them 
>>> into the
>>> NVRAM location of the emulated TPM.
>>>
>>> Then, libvirt will automatically start the swtpm TPM emulator using 
>>> the `swtpm`
>>> executable.
>>>
>>> Once the VM terminates, libvirt uses the swtpm_ioctl executable to 
>>> gracefully
>>> shut down the `swtpm` in case it is still running (QEMU did not send 
>>> shutdown)
>>> or clean up the socket file.
>>>
>>> The above mentioned executables must be found in the PATH.
>>>
>>> The executables can either be run as root or started as root and 
>>> switch to
>>> the tss user. The requirement for the tss user comes through 'tcsd', 
>>> which
>>> is used for the simulation of the manufacturing. Which user is used 
>>> can be
>>> configured through qemu.conf.
>>>
>>> The swtpm writes out state into files. The state is kept in 
>>> /var/lib/libvirt/tpm:
>>>
>>> [root@localhost libvirt]# ls -lZ | grep tpm
>>>
>>> drwx--x--x. 7 root root unconfined_u:object_r:virt_var_lib_t:s0 4096 
>>> Apr  5 16:22 tpm
>>>
>>> The directory /var/lib/libvirt/tpm maintains per-TPM state 
>>> directories but
>>> also hosts the UnixIO socket of running swtpms, which QEMU uses for 
>>> communicating
>>> with them. At this point only the socket file is labeled properly 
>>> and made accessible
>>> for QEMU, which runs under the qemu user:
>> /var/lib is for persistent state while /var/run is for transient
>> state, so I think sockets should be under /var/run instead.
>
> /var/run/libvirt/qemu then ?

I now moved it into this neighborhood, which seems good due to the 
existing permissions:

# ls -lZ domain-1-testvm/
total 4
-rw-------. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c40,c612 32 
Apr  6 09:55 master-key.aes
srwxrwxr-x. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c40,c612 0 
Apr  6 09:55 monitor.sock
srw-------. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c40,c612 0 
Apr  6 09:55 swtpm.sock

     Stefan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] tpm: Add support for external swtpm TPM emulator
Posted by Daniel P. Berrangé 7 years, 1 month ago
On Fri, Apr 06, 2018 at 07:23:49AM -0400, Stefan Berger wrote:
> On 04/06/2018 04:26 AM, Daniel P. Berrangé wrote:
> > On Thu, Apr 05, 2018 at 05:56:02PM -0400, Stefan Berger wrote:
> > > This patch adds support for an external swtpm TPM emulator. The XML for
> > > this type of TPM looks as follows:
> > > 
> > >   <tpm model='tpm-tis'>
> > >     <backend type='emulator'/>
> > >   </tpm>


> > > +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> > > +        VIR_ERROR("Could not start 'swtpm'. exitstatus: %d\n"
> > > +                  "stderr: %s\n", exitstatus, errbuf);
> > > +        virReportError(VIR_ERR_INTERNAL_ERROR,
> > > +                       _("Could not start 'swtpm'. exitstatus: %d, "
> > > +                       "error: %s"), exitstatus, errbuf);
> > > +        goto error;
> > > +    }
> > > +
> > > +    /* sync the startup of the swtpm's Unix socket with the start of QEMU */
> > > +    if (virTPMTryConnect(tpm->data.emulator.source.data.nix.path,
> > > +                         3 * 1000 * 1000) < 0) {
> > > +        virReportError(VIR_ERR_INTERNAL_ERROR,
> > > +                       _("Could not connect to the swtpm on '%s'"),
> > > +                       tpm->data.emulator.source.data.nix.path);
> > > +        goto error;
> > > +    }
> > Ewww, this is really not nice to deal with startup races.
> > 
> > You're using the --daemon flag to swtpm, so swtpm should make sure
> > that it doesn't daemonize itself & let the parent exit, until the
> > UNIX socket is actually ready to access connections. Can we just
> > get swtpm fixed in this way.
> 
> It does that actually already and the socket file is there once it
> daemonizes itself.

If that's the case, why do you need this TryConnect busy waiting loop
at all ? It should be immediately available.


> > > +    cmd = virCommandNew(swtpm_path);
> > > +    if (!cmd)
> > > +        goto error;
> > > +
> > > +    virCommandClearCaps(cmd);
> > > +
> > > +    virCommandAddArgList(cmd, "socket", "--daemon", "--ctrl", NULL);
> > > +    virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600",
> > > +                           tpm->data.emulator.source.data.nix.path);
> > > +
> > > +    virCommandAddArg(cmd, "--tpmstate");
> > > +    virCommandAddArgFormat(cmd, "dir=%s", storagepath);
> > > +
> > > +    virCommandAddArg(cmd, "--log");
> > > +    virCommandAddArgFormat(cmd, "file=%s", logfile);
> > > +
> > > +    /* allow process to open logfile by root before dropping privileges */
> > > +    virCommandAllowCap(cmd, CAP_DAC_OVERRIDE);
> > Why can't we get have the log file be owned by the user that
> > swtpm runs as, instead of root ?
> 
> I would have to look at this particular capability again. I initially wanted
> to put the swtpm's log file also into /var/log/libvirt/qemu. It works nice
> of course when running swtpm as 'root' but not so much when running it as
> 'tss':
> 
> root@localhost tmp]$ sudo ls -l /var/log/libvirt/ | grep qemu
> drwx------. 2 root root 20480 Apr  5 16:14 qemu

Yeah the logs are owned by root these days, because they're not written by
qemu itself, instead virtlogd owns it.

> So where do we put the swtpm's log files? /var/log/libvirt/swtpm?

Yeah, probably best to have a separate directory

> Iirc the CAP_DAC_OVERRIDE became necessary when swtpm tries to append to the
> log that 'tss' owns but now libvirt runs it as 'root'. I think that was the
> reason I added it. One way to solve this would be to chown() the files
> before starting swtpm. Is that the solution?

I'd think a separate dir is best

> 
> 
> > 
> > > +    if (swtpm_user > 0) {
> > > +        virCommandAddArg(cmd, "--runas");
> > > +        virCommandAddArgFormat(cmd, "%u", swtpm_user);
> > > +        virCommandAllowCap(cmd, CAP_SETGID);
> > > +        virCommandAllowCap(cmd, CAP_SETUID);
> > > +    }
> > Then we could tell virCommand to set the UID/GID before it even
> > executes this, and not use -runas, thus avoiding the need to
> > allow theses capabilities.
> 
> And the directory it writes the logs in would have to have ownership of
> either root:root or tss:root (or tss:tss), depending on how libvirt is
> currently configured? Again chown() the dir before starting?
> 
> > 
> > 
> > > +/*
> > > + * virTPMStopEmulator
> > > + * @tpm: TPM definition
> > > + * @vmuuid: the UUID of the VM
> > > + * @verbose: whether to report errors
> > > + *
> > > + * Gracefully stop the swptm
> > > + */
> > > +void
> > > +virTPMStopEmulator(virDomainTPMDefPtr tpm, const unsigned char *vmuuid,
> > > +                   bool verbose)
> > > +{
> > > +    virCommandPtr cmd;
> > > +    int exitstatus;
> > > +    char *pathname;
> > > +    char *errbuf = NULL;
> > > +
> > > +    (void)vmuuid;
> > > +    if (virTPMEmulatorInit() < 0)
> > > +        return;
> > > +
> > > +    if (!(pathname = virTPMCreateEmulatorSocket(vmuuid)))
> > > +        return;
> > > +
> > > +    cmd = virCommandNew(swtpm_ioctl);
> > > +    if (!cmd) {
> > > +        VIR_FREE(pathname);
> > > +        return;
> > > +    }
> > > +
> > > +    virCommandAddArgList(cmd, "--unix", pathname, "-s", NULL);
> > > +
> > > +    virCommandSetErrorBuffer(cmd, &errbuf);
> > > +
> > > +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> > > +        if (verbose)
> > > +            VIR_ERROR(_("Could not run swtpm_ioctl -s '%s'."
> > > +                      " existstatus: %d\nstderr: %s"),
> > > +                      swtpm_ioctl, exitstatus, errbuf);
> > > +    }
> > > +
> > > +    virCommandFree(cmd);
> > I would feel better if we just directly killed the process - with
> > this approach if something goes wrong with swtpm it may never
> > respond to this request and stay running.
> 
> swtpm can write a pidfile. I am only adding this later in this series.
> Problem is with --daemon libvirt doesn't know the pid of the swtpm anymore.

The other option is to not use --daemon, and let libvirt write the pid
file, but that introduces the race with socket path creation again
which is not good.


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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] tpm: Add support for external swtpm TPM emulator
Posted by Stefan Berger 7 years, 1 month ago
On 04/06/2018 10:12 AM, Daniel P. Berrangé wrote:
> On Fri, Apr 06, 2018 at 07:23:49AM -0400, Stefan Berger wrote:
>> On 04/06/2018 04:26 AM, Daniel P. Berrangé wrote:
>>> On Thu, Apr 05, 2018 at 05:56:02PM -0400, Stefan Berger wrote:
>>>> This patch adds support for an external swtpm TPM emulator. The XML for
>>>> this type of TPM looks as follows:
>>>>
>>>>    <tpm model='tpm-tis'>
>>>>      <backend type='emulator'/>
>>>>    </tpm>
>

>
>
>>>> +    cmd = virCommandNew(swtpm_path);
>>>> +    if (!cmd)
>>>> +        goto error;
>>>> +
>>>> +    virCommandClearCaps(cmd);
>>>> +
>>>> +    virCommandAddArgList(cmd, "socket", "--daemon", "--ctrl", NULL);
>>>> +    virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600",
>>>> +                           tpm->data.emulator.source.data.nix.path);
>>>> +
>>>> +    virCommandAddArg(cmd, "--tpmstate");
>>>> +    virCommandAddArgFormat(cmd, "dir=%s", storagepath);
>>>> +
>>>> +    virCommandAddArg(cmd, "--log");
>>>> +    virCommandAddArgFormat(cmd, "file=%s", logfile);
>>>> +
>>>> +    /* allow process to open logfile by root before dropping privileges */
>>>> +    virCommandAllowCap(cmd, CAP_DAC_OVERRIDE);
>>> Why can't we get have the log file be owned by the user that
>>> swtpm runs as, instead of root ?
>> I would have to look at this particular capability again. I initially wanted
>> to put the swtpm's log file also into /var/log/libvirt/qemu. It works nice
>> of course when running swtpm as 'root' but not so much when running it as
>> 'tss':
>>
>> root@localhost tmp]$ sudo ls -l /var/log/libvirt/ | grep qemu
>> drwx------. 2 root root 20480 Apr  5 16:14 qemu
> Yeah the logs are owned by root these days, because they're not written by
> qemu itself, instead virtlogd owns it.

[root@localhost log]# ls -lZ | grep libvirt
drwx------. 6 root      root 
system_u:object_r:virt_log_t:s0                   4096 Mar  1  2017 libvirt

Even /var/log/libvirt would not be accessible for the tss users.

>
>> So where do we put the swtpm's log files? /var/log/libvirt/swtpm?
> Yeah, probably best to have a separate directory

It would have to be /var/log/swtpm unless we change the permissions on 
/var/log/libvirt ... ?

>> Iirc the CAP_DAC_OVERRIDE became necessary when swtpm tries to append to the
>> log that 'tss' owns but now libvirt runs it as 'root'. I think that was the
>> reason I added it. One way to solve this would be to chown() the files
>> before starting swtpm. Is that the solution?
> I'd think a separate dir is best
>
>>
>>>> +    if (swtpm_user > 0) {
>>>> +        virCommandAddArg(cmd, "--runas");
>>>> +        virCommandAddArgFormat(cmd, "%u", swtpm_user);
>>>> +        virCommandAllowCap(cmd, CAP_SETGID);
>>>> +        virCommandAllowCap(cmd, CAP_SETUID);
>>>> +    }
>>> Then we could tell virCommand to set the UID/GID before it even
>>> executes this, and not use -runas, thus avoiding the need to
>>> allow theses capabilities.
>> And the directory it writes the logs in would have to have ownership of
>> either root:root or tss:root (or tss:tss), depending on how libvirt is
>> currently configured? Again chown() the dir before starting?
>>
>>>
>>>> +/*
>>>> + * virTPMStopEmulator
>>>> + * @tpm: TPM definition
>>>> + * @vmuuid: the UUID of the VM
>>>> + * @verbose: whether to report errors
>>>> + *
>>>> + * Gracefully stop the swptm
>>>> + */
>>>> +void
>>>> +virTPMStopEmulator(virDomainTPMDefPtr tpm, const unsigned char *vmuuid,
>>>> +                   bool verbose)
>>>> +{
>>>> +    virCommandPtr cmd;
>>>> +    int exitstatus;
>>>> +    char *pathname;
>>>> +    char *errbuf = NULL;
>>>> +
>>>> +    (void)vmuuid;
>>>> +    if (virTPMEmulatorInit() < 0)
>>>> +        return;
>>>> +
>>>> +    if (!(pathname = virTPMCreateEmulatorSocket(vmuuid)))
>>>> +        return;
>>>> +
>>>> +    cmd = virCommandNew(swtpm_ioctl);
>>>> +    if (!cmd) {
>>>> +        VIR_FREE(pathname);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    virCommandAddArgList(cmd, "--unix", pathname, "-s", NULL);
>>>> +
>>>> +    virCommandSetErrorBuffer(cmd, &errbuf);
>>>> +
>>>> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
>>>> +        if (verbose)
>>>> +            VIR_ERROR(_("Could not run swtpm_ioctl -s '%s'."
>>>> +                      " existstatus: %d\nstderr: %s"),
>>>> +                      swtpm_ioctl, exitstatus, errbuf);
>>>> +    }
>>>> +
>>>> +    virCommandFree(cmd);
>>> I would feel better if we just directly killed the process - with
>>> this approach if something goes wrong with swtpm it may never
>>> respond to this request and stay running.
>> swtpm can write a pidfile. I am only adding this later in this series.
>> Problem is with --daemon libvirt doesn't know the pid of the swtpm anymore.
> The other option is to not use --daemon, and let libvirt write the pid
> file, but that introduces the race with socket path creation again
> which is not good.

Sounds like we should leave this as it is? Unless swtpm was broken, 
there shouldn't be a reason why the we wouldn't be able to shut down 
swtpm by sending a command to it. The socket and its directory must not 
have disappeared of course.

   Stefan

>
>
> Regards,
> Daniel


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] tpm: Add support for external swtpm TPM emulator
Posted by Daniel P. Berrangé 7 years, 1 month ago
On Fri, Apr 06, 2018 at 10:49:23AM -0400, Stefan Berger wrote:
> On 04/06/2018 10:12 AM, Daniel P. Berrangé wrote:
> > On Fri, Apr 06, 2018 at 07:23:49AM -0400, Stefan Berger wrote:
> > > On 04/06/2018 04:26 AM, Daniel P. Berrangé wrote:
> > > > On Thu, Apr 05, 2018 at 05:56:02PM -0400, Stefan Berger wrote:
> > > > > This patch adds support for an external swtpm TPM emulator. The XML for
> > > > > this type of TPM looks as follows:
> > > > > 
> > > > >    <tpm model='tpm-tis'>
> > > > >      <backend type='emulator'/>
> > > > >    </tpm>
> > 
> 
> > 
> > 
> > > > > +    cmd = virCommandNew(swtpm_path);
> > > > > +    if (!cmd)
> > > > > +        goto error;
> > > > > +
> > > > > +    virCommandClearCaps(cmd);
> > > > > +
> > > > > +    virCommandAddArgList(cmd, "socket", "--daemon", "--ctrl", NULL);
> > > > > +    virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600",
> > > > > +                           tpm->data.emulator.source.data.nix.path);
> > > > > +
> > > > > +    virCommandAddArg(cmd, "--tpmstate");
> > > > > +    virCommandAddArgFormat(cmd, "dir=%s", storagepath);
> > > > > +
> > > > > +    virCommandAddArg(cmd, "--log");
> > > > > +    virCommandAddArgFormat(cmd, "file=%s", logfile);
> > > > > +
> > > > > +    /* allow process to open logfile by root before dropping privileges */
> > > > > +    virCommandAllowCap(cmd, CAP_DAC_OVERRIDE);
> > > > Why can't we get have the log file be owned by the user that
> > > > swtpm runs as, instead of root ?
> > > I would have to look at this particular capability again. I initially wanted
> > > to put the swtpm's log file also into /var/log/libvirt/qemu. It works nice
> > > of course when running swtpm as 'root' but not so much when running it as
> > > 'tss':
> > > 
> > > root@localhost tmp]$ sudo ls -l /var/log/libvirt/ | grep qemu
> > > drwx------. 2 root root 20480 Apr  5 16:14 qemu
> > Yeah the logs are owned by root these days, because they're not written by
> > qemu itself, instead virtlogd owns it.
> 
> [root@localhost log]# ls -lZ | grep libvirt
> drwx------. 6 root      root system_u:object_r:virt_log_t:s0
> 4096 Mar  1  2017 libvirt
> 
> Even /var/log/libvirt would not be accessible for the tss users.
> 
> > 
> > > So where do we put the swtpm's log files? /var/log/libvirt/swtpm?
> > Yeah, probably best to have a separate directory
> 
> It would have to be /var/log/swtpm unless we change the permissions on
> /var/log/libvirt ... ?

/var/log/swtpm is reasonable imho

> > > > I would feel better if we just directly killed the process - with
> > > > this approach if something goes wrong with swtpm it may never
> > > > respond to this request and stay running.
> > > swtpm can write a pidfile. I am only adding this later in this series.
> > > Problem is with --daemon libvirt doesn't know the pid of the swtpm anymore.
> > The other option is to not use --daemon, and let libvirt write the pid
> > file, but that introduces the race with socket path creation again
> > which is not good.
> 
> Sounds like we should leave this as it is? Unless swtpm was broken, there
> shouldn't be a reason why the we wouldn't be able to shut down swtpm by
> sending a command to it. The socket and its directory must not have
> disappeared of course.

Agreed.

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] tpm: Add support for external swtpm TPM emulator
Posted by Stefan Berger 7 years, 1 month ago
On 04/06/2018 10:54 AM, Daniel P. Berrangé wrote:
> On Fri, Apr 06, 2018 at 10:49:23AM -0400, Stefan Berger wrote:
>
>>>>> I would feel better if we just directly killed the process - with
>>>>> this approach if something goes wrong with swtpm it may never
>>>>> respond to this request and stay running.
>>>> swtpm can write a pidfile. I am only adding this later in this series.
>>>> Problem is with --daemon libvirt doesn't know the pid of the swtpm anymore.
>>> The other option is to not use --daemon, and let libvirt write the pid
>>> file, but that introduces the race with socket path creation again
>>> which is not good.
>> Sounds like we should leave this as it is? Unless swtpm was broken, there
>> shouldn't be a reason why the we wouldn't be able to shut down swtpm by
>> sending a command to it. The socket and its directory must not have
>> disappeared of course.
> Agreed.

I reworked this patch series quite a bit. Primarily in regards to the 
directories for where the data, socket, logfile, and pidfiles are 
stored. At the moment I need the following two additional SELinux rules 
for svirt on Fedora 23 (old).

allow svirt_t virtd_t:fifo_file write;
allow svirt_t virtd_t:process sigchld;

Not sure where I can find the sources for the policy, but maybe there's 
a more recent version that already has it?

Should this first patch be split? Take out the XML parser and generator ?

Regards,
    Stefan

>
> Regards,
> Daniel


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list