1. Patchew
  2. Libvirt
  3. View series

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

Stefan Berger posted 1 patch 2 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211008144405.2792979-1-stefanb@linux.ibm.com
There is a newer version of this series
src/qemu/qemu_tpm.c | 39 +++++++++++++++++++++++++++++++++++++++
src/util/virtpm.c   |  1 +
src/util/virtpm.h   |  1 +
3 files changed, 41 insertions(+)
[PATCH v3] qemu: tpm: Run swtpm_setup --create-config-files in session mode
Posted by Stefan Berger 2 years, 6 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>

---
v3:
  - Removed logfile parameter

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;
+
+    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",
+                          swtpm_setup, exitstatus);
+        return -1;
+    }
+
+    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
Re: [PATCH v3] qemu: tpm: Run swtpm_setup --create-config-files in session mode
Posted by Marc-André Lureau 2 years, 6 months ago 
Hi

On Fri, Oct 8, 2021 at 6:44 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>
>
> ---
> v3:
>   - Removed logfile parameter
>

I guess it's redirected to libvirt log then?


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

+
> +    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",
> +                          swtpm_setup, exitstatus);
> +        return -1;
> +    }
> +
> +    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
>
>
Re: [PATCH v3] qemu: tpm: Run swtpm_setup --create-config-files in session mode
Posted by Stefan Berger 2 years, 6 months ago 
On 10/8/21 12:33 PM, Marc-André Lureau wrote:
> Hi On Fri, Oct 8, 2021 at 6:44 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 
> ZjQcmQRYFpfptBannerStart
> This Message Is From an External Sender
> This message came from outside your organization.
> ZjQcmQRYFpfptBannerEnd
> Hi
>
> On Fri, Oct 8, 2021 at 6:44 PM Stefan Berger <stefanb@linux.ibm.com 
> <mailto: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
>     <https://bugzilla.redhat.com/show_bug.cgi?id=2010649>
>
>     Signed-off-by: Stefan Berger <stefanb@linux.ibm.com
>     <mailto:stefanb@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 ?


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


>
>     +
>     +    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
>
Re: [PATCH v3] qemu: tpm: Run swtpm_setup --create-config-files in session mode
Posted by Marc-André Lureau 2 years, 6 months ago 
Hi

On Fri, Oct 8, 2021 at 10:31 PM Stefan Berger <stefanb@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@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>
>>
>> ---
>> 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
>>
>>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.