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

Stefan Berger posted 1 patch 2 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211008135635.1116549-1-stefanb@linux.ibm.com
There is a newer version of this series
src/qemu/qemu_tpm.c | 43 +++++++++++++++++++++++++++++++++++++++++++
src/util/virtpm.c   |  1 +
src/util/virtpm.h   |  1 +
3 files changed, 45 insertions(+)
[PATCH v2] qemu: tpm: Run swtpm_setup --create-config-files in session mode
Posted by Stefan Berger 2 years, 5 months ago
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@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);
+        return -1;
+    }
+
+    return 0;
+}
+
+
 /*
  * qemuTPMEmulatorRunSetup
  *
@@ -432,6 +472,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
                                  "this requires privileged mode for a "
                                  "TPM 1.2\n"), 0600);
 
+    if (!privileged && qemuTPMCreateConfigFiles(logfile))
+        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

Re: [PATCH v2] qemu: tpm: Run swtpm_setup --create-config-files in session mode
Posted by Marc-André Lureau 2 years, 5 months ago
On Fri, Oct 8, 2021 at 5:56 PM Stefan Berger <stefanb@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@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);
>

logfile isn't used, it seems

+        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  /*
>   * qemuTPMEmulatorRunSetup
>   *
> @@ -432,6 +472,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>                                   "this requires privileged mode for a "
>                                   "TPM 1.2\n"), 0600);
>
> +    if (!privileged && qemuTPMCreateConfigFiles(logfile))
> +        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
>
>
lgtm otherwise
Re: [PATCH v2] qemu: tpm: Run swtpm_setup --create-config-files in session mode
Posted by Daniel P. Berrangé 2 years, 5 months ago
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 patch addresses the issue raised in
> https://bugzilla.redhat.com/show_bug.cgi?id=2010649
> 
> Signed-off-by: Stefan Berger <stefanb@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.

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.

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 :|

Re: [PATCH v2] qemu: tpm: Run swtpm_setup --create-config-files in session mode
Posted by Stefan Berger 2 years, 5 months ago
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@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.


>
> 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?


>
> Regards,
> Daniel


Re: [PATCH v2] qemu: tpm: Run swtpm_setup --create-config-files in session mode
Posted by Daniel P. Berrangé 2 years, 5 months ago
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@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 :|

Re: [PATCH v2] qemu: tpm: Run swtpm_setup --create-config-files in session mode
Posted by Stefan Berger 2 years, 5 months ago
On 10/8/21 10:43 AM, Daniel P. Berrangé wrote:
>
> This error path will trigger preventing use of the TPM, even if
> the user has manually setup the config themselves.
>
> Why aren't you running /usr/share/swtpm/swtpm-create-user-config-files
> instead which is what I see does exist on Fedora today.

That one will exit with error code '1' if any one file exists:

# /usr/share/swtpm/swtpm-create-user-config-files
File /home/stefanb/.config/swtpm_setup.conf already exists. Refusing to 
overwrite.
# echo $?
1

It wasn't designed to be run by libvirt but by the user on the command line.


    Stefan


>
> RHEL-8 has even older swtpm than Fedora.
>
> Regards,
> Daniel