From e3487aab5319df05c5a06a83e4d3e4a87c1e51a9 Mon Sep 17 00:00:00 2001

Message-Id: <e3487aab5319df05c5a06a83e4d3e4a87c1e51a9@dist-git>

From: Vasiliy Ulyanov <vulyanov@suse.de>

Date: Wed, 2 Feb 2022 17:28:16 +0100

Subject: [PATCH] qemu: tpm: Get swtpm pid without binary validation

Access to /proc/[pid]/exe may be restricted in certain environments (e.g.

in containers) and any attempt to stat(2) or readlink(2) the file will

result in 'permission denied' error if the calling process does not have

CAP_SYS_PTRACE capability. According to proc(5) manpage:

Permission to dereference or read (readlink(2)) this symbolic link is

governed by a ptrace access mode PTRACE_MODE_READ_FSCREDS check; see

ptrace(2).

The binary validation in virPidFileReadPathIfAlive may fail with EACCES.

Therefore instead do only the check that the pidfile is locked by the

correct process. To ensure this is always the case the daemonization and

pidfile handling of the swtpm command is now controlled by libvirt.

Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

(cherry picked from commit a9c500d2b50c5c041a1bb6ae9724402cf1cec8fe)

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2152188

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

---

 src/qemu/qemu_tpm.c | 94 ++++++++++++++++++++++++++-------------------

 1 file changed, 54 insertions(+), 40 deletions(-)

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c

index 7e7b01768e..9c5d1ffed4 100644

--- a/src/qemu/qemu_tpm.c

+++ b/src/qemu/qemu_tpm.c

@@ -44,6 +44,7 @@

 #include "qemu_tpm.h"

 #include "virtpm.h"

 #include "virsecret.h"

+#include "virtime.h"

 #define VIR_FROM_THIS VIR_FROM_NONE

@@ -258,13 +259,13 @@ qemuTPMEmulatorGetPid(const char *swtpmStateDir,

                       const char *shortName,

                       pid_t *pid)

 {

-    g_autofree char *swtpm = virTPMGetSwtpm();

     g_autofree char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir,

                                                                 shortName);

+

     if (!pidfile)

         return -1;

-    if (virPidFileReadPathIfAlive(pidfile, pid, swtpm) < 0)

+    if (virPidFileReadPathIfLocked(pidfile, pid) < 0)

         return -1;

     return 0;

@@ -660,9 +661,6 @@ qemuTPMEmulatorReconfigure(const char *storagepath,

  * @privileged: whether we are running in privileged mode

  * @swtpm_user: The uid for the swtpm to run as (drop privileges to from root)

  * @swtpm_group: The gid for the swtpm to run as

- * @swtpmStateDir: the directory where swtpm writes the pid file and creates the

- *                 Unix socket

- * @shortName: the short name of the VM

  * @incomingMigration: whether we have an incoming migration

  *

  * Create the virCommand use for starting the emulator

@@ -676,13 +674,10 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,

                             bool privileged,

                             uid_t swtpm_user,

                             gid_t swtpm_group,

-                            const char *swtpmStateDir,

-                            const char *shortName,

                             bool incomingMigration)

 {

     g_autoptr(virCommand) cmd = NULL;

     bool created = false;

-    g_autofree char *pidfile = NULL;

     g_autofree char *swtpm = virTPMGetSwtpm();

     int pwdfile_fd = -1;

     int migpwdfile_fd = -1;

@@ -721,7 +716,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,

     virCommandClearCaps(cmd);

-    virCommandAddArgList(cmd, "socket", "--daemon", "--ctrl", NULL);

+    virCommandAddArgList(cmd, "socket", "--ctrl", NULL);

     virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600",

                            tpm->data.emulator.source->data.nix.path);

@@ -748,12 +743,6 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,

         break;

     }

-    if (!(pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir, shortName)))

-        goto error;

-

-    virCommandAddArg(cmd, "--pid");

-    virCommandAddArgFormat(cmd, "file=%s", pidfile);

-

     if (tpm->data.emulator.hassecretuuid) {

         if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD)) {

             virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,

@@ -904,12 +893,14 @@ qemuExtTPMStartEmulator(virQEMUDriver *driver,

                         bool incomingMigration)

 {

     g_autoptr(virCommand) cmd = NULL;

-    int exitstatus = 0;

-    g_autofree char *errbuf = NULL;

+    VIR_AUTOCLOSE errfd = -1;

     g_autoptr(virQEMUDriverConfig) cfg = NULL;

     g_autofree char *shortName = virDomainDefGetShortName(vm->def);

-    int cmdret = 0, timeout, rc;

-    pid_t pid;

+    g_autofree char *pidfile = NULL;

+    virTimeBackOffVar timebackoff;

+    const unsigned long long timeout = 1000; /* ms */

+    int cmdret = 0;

+    pid_t pid = -1;

     if (!shortName)

         return -1;

@@ -923,48 +914,71 @@ qemuExtTPMStartEmulator(virQEMUDriver *driver,

                                             driver->privileged,

                                             cfg->swtpm_user,

                                             cfg->swtpm_group,

-                                            cfg->swtpmStateDir, shortName,

                                             incomingMigration)))

         return -1;

     if (qemuExtDeviceLogCommand(driver, vm, cmd, "TPM Emulator") < 0)

         return -1;

-    virCommandSetErrorBuffer(cmd, &errbuf);

+    if (!(pidfile = qemuTPMEmulatorCreatePidFilename(cfg->swtpmStateDir, shortName)))

+        return -1;

+

+    virCommandDaemonize(cmd);

+    virCommandSetPidFile(cmd, pidfile);

+    virCommandSetErrorFD(cmd, &errfd);

     if (qemuSecurityStartTPMEmulator(driver, vm, cmd,

                                      cfg->swtpm_user, cfg->swtpm_group,

-                                     &exitstatus, &cmdret) < 0)

+                                     NULL, &cmdret) < 0)

         return -1;

-    if (cmdret < 0 || exitstatus != 0) {

-        virReportError(VIR_ERR_INTERNAL_ERROR,

-                       _("Could not start 'swtpm'. exitstatus: %d, "

-                         "error: %s"), exitstatus, errbuf);

-        return -1;

+    if (cmdret < 0) {

+        /* virCommandRun() hidden in qemuSecurityStartTPMEmulator()

+         * already reported error. */

+        goto error;

     }

-    /* check that the swtpm has written its pid into the file */

-    timeout = 1000; /* ms */

-    while (timeout > 0) {

-        rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid;;

-        if (rc < 0) {

-            timeout -= 50;

-            g_usleep(50 * 1000);

+    if (virPidFileReadPath(pidfile, &pid) < 0) {

+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

+                       _("swtpm didn't show up"));

+        goto error;

+    }

+

+    if (virTimeBackOffStart(&timebackoff, 1, timeout) < 0)

+        goto error;

+    while (virTimeBackOffWait(&timebackoff)) {

+        char errbuf[1024] = { 0 };

+

+        if (virFileExists(tpm->data.emulator.source->data.nix.path))

+            break;

+

+        if (virProcessKill(pid, 0) == 0)

             continue;

+

+        if (saferead(errfd, errbuf, sizeof(errbuf) - 1) < 0) {

+            virReportSystemError(errno, "%s",

+                                 _("swtpm died unexpectedly"));

+        } else {

+            virReportError(VIR_ERR_OPERATION_FAILED,

+                           _("swtpm died and reported: %s"), errbuf);

         }

-        if (rc == 0 && pid == (pid_t)-1)

-            goto error;

-        break;

+        goto error;

     }

-    if (timeout <= 0)

+

+    if (!virFileExists(tpm->data.emulator.source->data.nix.path)) {

+        virReportError(VIR_ERR_OPERATION_TIMEOUT, "%s",

+                       _("swtpm socket did not show up"));

         goto error;

+    }

     return 0;

  error:

-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

-                   _("swtpm failed to start"));

+    virCommandAbort(cmd);

+    if (pid >= 0)

+        virProcessKillPainfully(pid, true);

+    if (pidfile)

+        unlink(pidfile);

     return -1;

 }

-- 

2.39.0