[libvirt] [PATCH 06/12] qemu: Extend QEMU with external TPM support

[Stefan Berger]

On 05/23/2018 11:41 AM, Ján Tomko wrote:
> On Tue, May 22, 2018 at 04:44:47PM -0400, Stefan Berger wrote:
>> Implement functions for managing the storage of the external swtpm as 
>> well
>> as starting and stopping it. Also implement functions to use 
>> swtpm_setup,
>> which simulates the manufacturing of a TPM, which includes creation of
>> certificates for the device.
>>
>> Further, the external TPM needs storage on the host that we need to set
>> up before it can be run. We can clean up the host once the domain is
>> undefined.
>>
>> This patch also implements a small layer for external device support 
>> that
>> calls into the TPM device layer if a domain has an attached TPM. This is
>> the layer we will wire up later on.
>>
>
> Later on meaning in other patch series? Adding it for just one device
> seems excessive, but that might be my personal opinion.
>
>> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
>> Reviewed-by: John Ferlan <jferlan at redhat.com>
>> ---
>> src/qemu/Makefile.inc.am  |   4 +
>> src/qemu/qemu_domain.c    |   2 +
>> src/qemu/qemu_extdevice.c | 154 ++++++++++
>> src/qemu/qemu_extdevice.h |  53 ++++
>> src/qemu/qemu_process.c   |  12 +
>> src/qemu/qemu_tpm.c       | 751 
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> src/qemu/qemu_tpm.h       |  50 +++
>> 7 files changed, 1026 insertions(+)
>> create mode 100644 src/qemu/qemu_extdevice.c
>> create mode 100644 src/qemu/qemu_extdevice.h
>> create mode 100644 src/qemu/qemu_tpm.c
>> create mode 100644 src/qemu/qemu_tpm.h
>>
>
>> +/*
>> + * virtTPMGetTPMStorageDir:
>> + *
>> + * @storagepath: directory for swtpm's persistent state
>> + *
>> + * Derive the 'TPMStorageDir' from the storagepath by searching
>> + * for the last '/'.
>> + */
>> +static char *
>> +qemuTPMGetTPMStorageDir(const char *storagepath)
>> +{
>> +    const char *tail = strrchr(storagepath, '/');
>> +    char *path = NULL;
>> +
>> +    if (!tail) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("Could not get tail of storagedir %s"),
>> +                       storagepath);
>> +        return NULL;
>> +    }
>> +    ignore_value(VIR_STRNDUP(path, storagepath, tail - storagepath));
>> +
>> +    return path;
>> +}
>
> In other places we already use mdir_name from gnulib for this
> functionality.


I didn't know about this API. I will change it to that.


>
>> +/*
>> + * qemuTPMCreateEmulatorStorage
>> + *
>> + * @storagepath: directory for swtpm's persistent state
>> + * @created: a pointer to a bool that will be set to true if the
>> + *           storage was created because it did not exist yet
>
> Can't the caller call virFileExists and act accordingly?


We could, except that we have to call this function since it is 
adjusting access rights to files for the swtpm_user and swtpm_group.


>
>> + * @swtpm_user: The uid that needs to be able to access the directory
>> + * @swtpm_group: The gid that needs to be able to access the directory
>> + *
>> + * Unless the storage path for the swtpm for the given VM
>> + * already exists, create it and make it accessible for the given 
>> userid.
>> + * Adapt ownership of the directory and all swtpm's state files there.
>> + */
>
> [...]
>
>> +static int
>> +qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm,
>> +                           const char *logDir,
>> +                           const char *vmname,
>> +                           uid_t swtpm_user,
>> +                           gid_t swtpm_group,
>> +                           const char *swtpmStateDir,
>> +                           uid_t qemu_user,
>> +                           const char *shortName)
>> +{
>> +    int ret = -1;
>> +
>> +    if (qemuTPMEmulatorInit() < 0)
>> +        return -1;
>> +
>> +    /* create log dir ... allow 'tss' user to cd into it */
>> +    if (virFileMakePathWithMode(logDir, 0711) < 0)
>> +        return -1;
>> +
>> +    /* ... and adjust ownership */
>> +    if (virDirCreate(logDir, 0730, swtpm_user, swtpm_group,
>> +                     VIR_DIR_CREATE_ALLOW_EXIST) < 0)
>> +        goto cleanup;
>> +
>> +    /* create logfile name ... */
>> +    if (!tpm->data.emulator.logfile &&
>> +        virAsprintf(&tpm->data.emulator.logfile, "%s/%s-swtpm.log",
>> +                    logDir, vmname) < 0)
>
> This should also use shortName.


The shortName has the ID of the domain in the name. So for short-lived 
logs I would say yes. Though this should be a log like the one for the 
VM that gets appended to every time the VM restarts. I'd rather not 
change this.


>
>> +        goto cleanup;
>> +
>> +    /* ... and make sure it can be accessed by swtpm_user */
>> +    if (virFileExists(tpm->data.emulator.logfile) &&
>> +        chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 
>> 0) {
>> +        virReportSystemError(errno,
>> +                             _("Could not chown on swtpm logfile %s"),
>> +                             tpm->data.emulator.logfile);
>> +        goto cleanup;
>> +    }
>> +
>> +    /*
>> +      create our swtpm state dir ...
>> +      - QEMU user needs to be able to access the socket there
>> +      - swtpm group needs to be able to create files there
>> +      - in privileged mode 0570 would be enough, for non-privileged 
>> mode
>> +        we need 0770
>> +    */
>> +    if (virDirCreate(swtpmStateDir, 0770, qemu_user, swtpm_group,
>> +                     VIR_DIR_CREATE_ALLOW_EXIST) < 0)
>> +        goto cleanup;
>> +
>> +    /* create the socket filename */
>> +    if (!tpm->data.emulator.source.data.nix.path &&
>> +        !(tpm->data.emulator.source.data.nix.path =
>> +          qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName)))
>> +        goto cleanup;
>> +    tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
>> +
>> +    ret = 0;
>> +
>> + cleanup:
>> +
>> +    return ret;
>> +}
>> +
>
> [...]
>
>> +static int
>> +qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
>> +                        virDomainDefPtr def,
>> +                        qemuDomainLogContextPtr logCtxt)
>> +{
>> +    int ret = -1;
>> +    virCommandPtr cmd = NULL;
>> +    int exitstatus;
>> +    char *errbuf = NULL;
>> +    virQEMUDriverConfigPtr cfg;
>> +    virDomainTPMDefPtr tpm = def->tpm;
>> +    char *shortName = virDomainDefGetShortName(def);
>> +
>> +    if (!shortName)
>> +        return -1;
>> +
>> +    cfg = virQEMUDriverGetConfig(driver);
>> +
>> +    /* stop any left-over TPM emulator for this VM */
>> +    qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName);
>> +
>> +    if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, def->name, def->uuid,
>> + driver->privileged,
>> +                                            cfg->swtpm_user,
>> + cfg->swtpm_group)))
>> +        goto cleanup;
>> +
>> +    if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0)
>> +        goto cleanup;
>> +
>> +    virCommandSetErrorBuffer(cmd, &errbuf);
>> +
>> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("Could not start 'swtpm'. exitstatus: %d, "
>> +                       "error: %s"), exitstatus, errbuf);
>
> Indentation is off here  ^

Fixed.

>
>> +        goto cleanup;
>> +    }
>> +
>> +    ret = 0;
>> +
>
> Jano


Thanks..

    Stefan