[PATCH v2] qemu: tpm: Run swtpm_setup --create-config-files in session mode

[Daniel P. Berrangé]

On Fri, Oct 08, 2021 at 10:51:24AM -0400, Stefan Berger wrote:
> 
> On 10/8/21 10:43 AM, Daniel P. Berrangé wrote:
> > On Fri, Oct 08, 2021 at 09:56:35AM -0400, Stefan Berger 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.
> > Fedora 34 only has v0.6.0 and so....
> 
> This is a new feature that will come out with v0.7.0.
> 
> 
> > 
> > > 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>
> > > 
> > > v2:
> > >    - fixed return code if swtpm_setup doesn't support the option
> > > ---
> > >   src/qemu/qemu_tpm.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > >   src/util/virtpm.c   |  1 +
> > >   src/util/virtpm.h   |  1 +
> > >   3 files changed, 45 insertions(+)
> > > 
> > > diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> > > index 100481503c..bf6c8e5ad5 100644
> > > --- a/src/qemu/qemu_tpm.c
> > > +++ b/src/qemu/qemu_tpm.c
> > > @@ -385,6 +385,46 @@ qemuTPMSetupEncryption(const unsigned char *secretuuid,
> > >       return virCommandSetSendBuffer(cmd, g_steal_pointer(&secret), secret_len);
> > >   }
> > > +
> > > +/*
> > > + * qemuTPMCreateConfigFiles: run swtpm_setup --create-config-files skip-if-exist
> > > + *
> > > + * @logfile: The file to write the log into; it must be writable
> > > + *           for the user given by userid or 'tss'
> > > + */
> > > +static int
> > > +qemuTPMCreateConfigFiles(const char *logfile)
> > > +{
> > > +    g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
> > > +    g_autoptr(virCommand) cmd = NULL;
> > > +    int exitstatus;
> > > +
> > > +    if (!swtpm_setup)
> > > +        return -1;
> > > +
> > > +    if (!virTPMSwtpmSetupCapsGet(
> > > +            VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES))
> > > +        return 0;
> > > +
> > > +    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; "
> > > +                         "Check error log '%s' for details."),
> > > +                          swtpm_setup, exitstatus, logfile);
> > This error path will trigger preventing use of the TPM, even if
> > the user has manually setup the config themselves.
> 
> skip-if-exists results in exit code 0 if any one of the 3 expected files
> exist.

No, you mis-understand me.  I have creates the cofnig by running
/usr/share/swtpm/swtpm-create-user-config-files.

Now this libvirt tries to invoke swtpm_setup --create-config-files skip-if-exists which fails to parse the command line, as I don't have version 0.7.0,
and thus prevents my VMs working

> > Why aren't you running /usr/share/swtpm/swtpm-create-user-config-files
> > instead which is what I see does exist on Fedora today.
> > 
> > RHEL-8 has even older swtpm than Fedora.
> 
> This patch will be backported then and not regarded as new feature?

We can't assume distros will backport new cli options like that. We need
to be able to work correctly with the version of swtpm that is shipped
in the various distros today.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|