Re: [PATCH 14/18] tpm: Use fd to pass password to swtpm_setup and swtpm

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

 



On 7/9/19 4:25 PM, Marc-André Lureau wrote:
On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx> wrote:
Allow vTPM state encryption when swtpm_setup and swtpm support
passing a passphrase using a file descriptor.

This patch enables the encryption of the vTPM state only. It does
not encrypt the state during migration, so the destination secret
does not need to have the same password at this point.
You could add that it is addressed in the following patch

Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
---
  src/libvirt_private.syms |   2 +
  src/qemu/qemu_tpm.c      | 101 ++++++++++++++++++++++++++++++++++++++-
  src/tpm/virtpm.c         |  16 +++++++
  src/tpm/virtpm.h         |   3 ++
  4 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d2045895a1..d693f7facb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1456,6 +1456,8 @@ virTPMEmulatorInit;
  virTPMGetSwtpm;
  virTPMGetSwtpmIoctl;
  virTPMGetSwtpmSetup;
+virTPMSwtpmCapsGet;
+virTPMSwtpmSetupCapsGet;


  # util/viralloc.h
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 2afa8db448..6e7d38b7e0 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -43,6 +43,8 @@
  #include "dirname.h"
  #include "qemu_tpm.h"
  #include "virtpm.h"
+#include "secret_util.h"
+#include "virtpm_conf.h"

  #define VIR_FROM_THIS VIR_FROM_NONE

@@ -372,6 +374,60 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm,
      return ret;
  }

+/*
+ * qemuTPMSetupEncryption
+ *
+ * @encryption: pointer to virStorageEncryption holding secret
+ *
+ * Returns file descriptor representing the read-end of a pipe.
+ * The passphrase can be read from this pipe. Returns < 0 in case
+ * of error.
+ *
+ * This function reads the passphrase and writes it into the
+ * write-end of a pipe so that the read-end of the pipe can be
+ * passed to the emulator for reading the passphrase from.
+ */
+static int
+qemuTPMSetupEncryption(virStorageEncryptionPtr encryption)
+{
+    int ret = -1;
+    int pipefd[2] = { -1, -1 };
+    virConnectPtr conn;
+    uint8_t *secret = NULL;
+    size_t secret_len;
+
+    conn = virGetConnectSecret();
+    if (!conn)
+        return -1;
+
+    if (virSecretGetSecretString(conn, &encryption->secrets[0]->seclookupdef,
+                                 VIR_SECRET_USAGE_TYPE_VTPM,
+                                 &secret, &secret_len) < 0)
+        goto error;
+
+    if (pipe(pipefd) == -1) {
+        virReportSystemError(errno, "%s",
+                             _("Unable to create pipe"));
+        goto error;
+    }
+
+    if (safewrite(pipefd[1], secret, secret_len) != secret_len)
+        goto error;
Hmm, I am not sure you can reliably buffer data on a pipe end and
close it before the other end read. Got any documentation pointer
about that?

I wasn't sure about this, either, but the man page says:

"[...] Data written to the write end of the pipe is
       buffered by the kernel until it is read from the read end of the
       pipe.  For further details, see pipe(7)"


http://man7.org/linux/man-pages/man2/pipe.2.html

From 'man 7 pipe': http://man7.org/linux/man-pages/man7/pipe.7.html

In Linux versions before 2.6.11, the capacity of a pipe was the same
       as the system page size (e.g., 4096 bytes on i386).  Since Linux
       2.6.11, the pipe capacity is 16 pages (i.e., 65,536 bytes in a system
       with a page size of 4096 bytes).  Since Linux 2.6.35, the default
       pipe capacity is 16 pages, but the capacity can be queried and set
       using the fcntl(2) F_GETPIPE_SZ and F_SETPIPE_SZ operations.  See
       fcntl(2) for more information.

I would say on Linux the size of the pipe is large enough for the purpose of passing a secret, or do we need to assume 65kb sized secrets? Code that may write to the pipe in virCommand after the fork() may never get exercised unless we were to change the capacity or initiate the writing to the pipe only after the fork() happened.

Which other systems is libvirt running on ?

https://unix.stackexchange.com/questions/11946/how-big-is-the-pipe-buffer

"Mac OS X, for example, uses a capacity of 16384 bytes by default, but can switch to 65336 byte capacities if large write are made to the pipe, or will switch to a capacity of a single system page if too much kernel memory is already being used by pipe buffers (see xnu/bsd/sys/pipe.h, and xnu/bsd/kern/sys_pipe.c; since these are from FreeBSD, the same behavior may happen there, too)."




+
+    ret = pipefd[0];
+
+ cleanup:
+    VIR_FREE(secret);
+    VIR_FORCE_CLOSE(pipefd[1]);
+    virObjectUnref(conn);
+
+    return ret;
+
+ error:
+    VIR_FORCE_CLOSE(pipefd[0]);
+
+    goto cleanup;
+}

  /*
   * qemuTPMEmulatorRunSetup
@@ -386,6 +442,7 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm,
   * @logfile: The file to write the log into; it must be writable
   *           for the user given by userid or 'tss'
   * @tpmversion: The version of the TPM, either a TPM 1.2 or TPM 2
+ * @encryption: pointer to virStorageEncryption holding secret
   *
   * Setup the external swtpm by creating endorsement key and
   * certificates for it.
@@ -398,13 +455,15 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
                          uid_t swtpm_user,
                          gid_t swtpm_group,
                          const char *logfile,
-                        const virDomainTPMVersion tpmversion)
+                        const virDomainTPMVersion tpmversion,
+                        virStorageEncryptionPtr encryption)
  {
      virCommandPtr cmd = NULL;
      int exitstatus;
      int ret = -1;
      char uuid[VIR_UUID_STRING_BUFLEN];
      char *vmid = NULL;
+    int pwdfile_fd = -1;

      if (!privileged && tpmversion == VIR_DOMAIN_TPM_VERSION_1_2)
          return virFileWriteStr(logfile,
@@ -434,6 +493,22 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
          break;
      }

+    if (encryption) {
+        if (!virTPMSwtpmSetupCapsGet(
+                VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD)) {
+            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+                _("%s does not support passing a passphrase using a file "
+                  "descriptor"), virTPMGetSwtpmSetup());
+            goto cleanup;
+        }
+        if ((pwdfile_fd = qemuTPMSetupEncryption(encryption)) < 0)
+            goto cleanup;
+
+        virCommandAddArg(cmd, "--pwdfile-fd");
+        virCommandAddArgFormat(cmd, "%d", pwdfile_fd);
+        virCommandAddArgList(cmd, "--cipher", "aes-256-cbc", NULL);
+        virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+    }

      virCommandAddArgList(cmd,
                           "--tpm-state", storagepath,
@@ -461,6 +536,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
   cleanup:
      VIR_FREE(vmid);
      virCommandFree(cmd);
+    VIR_FORCE_CLOSE(pwdfile_fd);

virCommandPassFD() doc says:
"The parent should cease using the @fd when this call completes"
Right, need to remove this.




      return ret;
  }
@@ -496,6 +572,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
      virCommandPtr cmd = NULL;
      bool created = false;
      char *pidfile;
+    int pwdfile_fd = -1;

      if (qemuTPMCreateEmulatorStorage(tpm->data.emulator.storagepath,
                                       &created, swtpm_user, swtpm_group) < 0)
@@ -504,7 +581,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
      if (created &&
          qemuTPMEmulatorRunSetup(tpm->data.emulator.storagepath, vmname, vmuuid,
                                  privileged, swtpm_user, swtpm_group,
-                                tpm->data.emulator.logfile, tpm->version) < 0)
+                                tpm->data.emulator.logfile, tpm->version,
+                                tpm->data.emulator.encryption) < 0)
          goto error;

      unlink(tpm->data.emulator.source.data.nix.path);
@@ -547,11 +625,30 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
      virCommandAddArgFormat(cmd, "file=%s", pidfile);
      VIR_FREE(pidfile);

+    if (tpm->data.emulator.encryption) {
+        if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD)) {
+            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+                  _("%s does not support passing passphrase via file descriptor"),
+                  virTPMGetSwtpm());
+            goto error;
+        }
+
+        pwdfile_fd = qemuTPMSetupEncryption(tpm->data.emulator.encryption);
+        if (pwdfile_fd < 0)
+            goto error;
+
+        virCommandAddArg(cmd, "--key");
+        virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc,kdf=pbkdf2",
+                               pwdfile_fd);
+        virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+    }
+
      return cmd;

   error:
      if (created)
          qemuTPMDeleteEmulatorStorage(tpm);
+    VIR_FORCE_CLOSE(pwdfile_fd);
same, and similarly in next patch


Will remove.

Thanks,

   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