[libvirt] [PATCH v7 15/19] tpm: Use fd to pass password to swtpm_setup and swtpm

[Stefan Berger]

On 7/26/19 6:47 AM, John Ferlan wrote:
>
> On 7/25/19 2:22 PM, Stefan Berger 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.
>>
>> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
>> Reviewed-by: Daniel P. Berrangé <berrange at redhat.com>
>> ---
>>   src/libvirt_private.syms |   2 +
>>   src/qemu/qemu_tpm.c      | 110 ++++++++++++++++++++++++++++++++++++++-
>>   src/util/virtpm.c        |  16 ++++++
>>   src/util/virtpm.h        |   3 ++
>>   4 files changed, 129 insertions(+), 2 deletions(-)
>>
> [...]
>
>>   /*
>>    * qemuTPMEmulatorRunSetup
>> @@ -387,6 +448,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.
>> @@ -399,7 +461,8 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>>                           uid_t swtpm_user,
>>                           gid_t swtpm_group,
>>                           const char *logfile,
>> -                        const virDomainTPMVersion tpmversion)
>> +                        const virDomainTPMVersion tpmversion,
>> +                        const unsigned char *secretuuid)
>>   {
>>       virCommandPtr cmd = NULL;
>>       int exitstatus;
>> @@ -407,6 +470,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>>       char uuid[VIR_UUID_STRING_BUFLEN];
>>       char *vmid = NULL;
>>       VIR_AUTOFREE(char *)swtpm_setup = virTPMGetSwtpmSetup();
>> +    VIR_AUTOCLOSE pwdfile_fd = -1;
>>   
>>       if (!swtpm_setup)
>>           return -1;
>> @@ -439,6 +503,23 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>>           break;
>>       }
>>   
>> +    if (secretuuid) {
>> +        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());
> Coverity complains since virTPMGetSwtpm() returns something that needs
> to be free'd.  I note above that @swtpm_setup is already set - is that
> what was meant here?


Right. Will send patch for it. Do you want a single patch or one for 
each issue?




>
>> +            goto cleanup;
>> +        }
>> +        if ((pwdfile_fd = qemuTPMSetupEncryption(secretuuid, cmd)) < 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);
>> +        pwdfile_fd = -1;
>> +    }
>>   
>>       virCommandAddArgList(cmd,
>>                            "--tpm-state", storagepath,
>> @@ -502,6 +583,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
>>       bool created = false;
>>       char *pidfile;
>>       VIR_AUTOFREE(char *) swtpm = virTPMGetSwtpm();
>> +    VIR_AUTOCLOSE pwdfile_fd = -1;
>> +    const unsigned char *secretuuid = NULL;
>>   
>>       if (!swtpm)
>>           return NULL;
>> @@ -510,10 +593,14 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
>>                                        &created, swtpm_user, swtpm_group) < 0)
>>           return NULL;
>>   
>> +    if (tpm->data.emulator.hassecretuuid)
>> +        secretuuid = tpm->data.emulator.secretuuid;
>> +
>>       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,
>> +                                secretuuid) < 0)
>>           goto error;
>>   
>>       unlink(tpm->data.emulator.source.data.nix.path);
>> @@ -556,6 +643,25 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
>>       virCommandAddArgFormat(cmd, "file=%s", pidfile);
>>       VIR_FREE(pidfile);
>>   
>> +    if (tpm->data.emulator.hassecretuuid) {
>> +        if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD)) {
>> +            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
>> +                  _("%s does not support passing passphrase via file descriptor"),
>> +                  virTPMGetSwtpm());
> Same, but @swtpm is used in this context


Gee.


    Stefan