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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxx>
---
  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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux