[PATCH v3] qemu: tpm: Run swtpm_setup --create-config-files in session mode
Marc-André Lureau
marcandre.lureau at redhat.com
Fri Oct 8 18:48:14 UTC 2021
Hi
On Fri, Oct 8, 2021 at 10:31 PM Stefan Berger <stefanb at linux.ibm.com> wrote:
>
> On 10/8/21 12:33 PM, Marc-André Lureau wrote:
>
> Hi
>
> On Fri, Oct 8, 2021 at 6:44 PM Stefan Berger <stefanb at linux.ibm.com>
> wrote:
>
>> Using swtpm v0.7.0 we can run swtpm_setup to create default config files
>> for swtpm_setup and swtpm-localca in session mode. Now a user can start
>> a VM with an attached TPM without having to run this program on the
>> command line before. This program needs to run once.
>>
>> This patch addresses the issue raised in
>> https://bugzilla.redhat.com/show_bug.cgi?id=2010649
>>
>> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
>>
>> ---
>> v3:
>> - Removed logfile parameter
>>
>
> I guess it's redirected to libvirt log then?
>
> So you are saying it should capture the stderr output ?
>
Something should, not sure what's the best practice today with libvirt.
Someone more familiar should say.
>
>> v2:
>> - fixed return code if swtpm_setup doesn't support the option
>> ---
>> src/qemu/qemu_tpm.c | 39 +++++++++++++++++++++++++++++++++++++++
>> src/util/virtpm.c | 1 +
>> src/util/virtpm.h | 1 +
>> 3 files changed, 41 insertions(+)
>>
>> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
>> index 100481503c..434c357c24 100644
>> --- a/src/qemu/qemu_tpm.c
>> +++ b/src/qemu/qemu_tpm.c
>> @@ -385,6 +385,42 @@ qemuTPMSetupEncryption(const unsigned char
>> *secretuuid,
>> return virCommandSetSendBuffer(cmd, g_steal_pointer(&secret),
>> secret_len);
>> }
>>
>> +
>> +/*
>> + * qemuTPMCreateConfigFiles: run swtpm_setup --create-config-files
>> skip-if-exist
>> + */
>> +static int
>> +qemuTPMCreateConfigFiles(void)
>> +{
>> + g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
>> + g_autoptr(virCommand) cmd = NULL;
>> + int exitstatus;
>> +
>> + if (!swtpm_setup)
>> + return -1;
>>
>
> I think what Daniel was saying is that this shouldn't fail suddenly for
> users with older swtpm that already have the configuration setup.
>
>
> The above only fails if swtm_setup is not installed, if that's what you
> mean. That's when swtpm_setup is NULL.
>
> If you run `swptm_setup --create-config-files skip-if-exist` when any one
> of the 3 config files already exist, it will do nothing and exit with 0.
>
>
>
Oh I see, I misread, looks correct then
> +
>> + if (!virTPMSwtpmSetupCapsGet(
>> + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES))
>> + return 0;
>>
>
> This is the case when swtpm_setup doesn't support --create-config-files.
> That's when the user has to use the old
> /usr/share/swtpm/swptm-create-user-config-files to create the config files,
> which will then be located at the same place that `swptm_setup
> --create-config-files skip-if-exist` would create them or check whether any
> one of them exist and skip.
>
> 3 config files will be created and if the user then removes 1 or 2 of them
> he created an invalid configuration and the start of the next VM will fail
> due to a bad configuration with missing config files. The config files
> would only be created again if all 3 were missing.
>
>
> +
>> + cmd = virCommandNew(swtpm_setup);
>> + if (!cmd)
>> + return -1;
>> +
>> + virCommandAddArgList(cmd, "--create-config-files", "skip-if-exist",
>> NULL);
>> + virCommandClearCaps(cmd);
>> +
>> + if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Could not run '%s' to create config files.
>> exitstatus: %d",
>> + swtpm_setup, exitstatus);
>> + return -1;
>>
> In the error case the stderr output of swtpm_setup should probably show up
> here?
>
>
> + }
>> +
>> + return 0;
>> +}
>> +
>> +
>> /*
>> * qemuTPMEmulatorRunSetup
>> *
>> @@ -432,6 +468,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>> "this requires privileged mode for a "
>> "TPM 1.2\n"), 0600);
>>
>> + if (!privileged && qemuTPMCreateConfigFiles())
>> + return -1;
>> +
>> cmd = virCommandNew(swtpm_setup);
>> if (!cmd)
>> return -1;
>> diff --git a/src/util/virtpm.c b/src/util/virtpm.c
>> index 1a567139b4..0f50de866c 100644
>> --- a/src/util/virtpm.c
>> +++ b/src/util/virtpm.c
>> @@ -45,6 +45,7 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature,
>> VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
>> VIR_TPM_SWTPM_SETUP_FEATURE_LAST,
>> "cmdarg-pwdfile-fd",
>> + "cmdarg-create-config-files",
>> );
>>
>> /**
>> diff --git a/src/util/virtpm.h b/src/util/virtpm.h
>> index d021a083b4..3bb03b3b33 100644
>> --- a/src/util/virtpm.h
>> +++ b/src/util/virtpm.h
>> @@ -38,6 +38,7 @@ typedef enum {
>>
>> typedef enum {
>> VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD,
>> + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES,
>>
>> VIR_TPM_SWTPM_SETUP_FEATURE_LAST
>> } virTPMSwtpmSetupFeature;
>> --
>> 2.31.1
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20211008/0e9b661f/attachment-0001.htm>
More information about the libvir-list
mailing list