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

On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb(a)linux.vnet.ibm.com&gt; 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(a)linux.ibm.com&gt; > --- > 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?

> + > + 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"

> 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