[libvirt] [PATCH 14/18] tpm: Use fd to pass password to swtpm_setup and swtpm
Stefan Berger
stefanb at linux.ibm.com
Wed Jul 10 11:44:15 UTC 2019
On 7/9/19 4:25 PM, Marc-André Lureau wrote:
> On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb at linux.vnet.ibm.com> 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 at linux.ibm.com>
>> ---
>> 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
More information about the libvir-list
mailing list