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

Stefan Berger posted 1 patch 1 week, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211013125713.592949-1-stefanb@linux.ibm.com
src/qemu/qemu_tpm.c | 42 ++++++++++++++++++++++++++++++++++++++++++
src/util/virtpm.c   |  1 +
src/util/virtpm.h   |  1 +
3 files changed, 44 insertions(+)

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

Posted by Stefan Berger 1 week, 2 days 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>

---
v4:
  - Append stderr output to virReportError if swtpm_setup fails

v3:
  - Removed logfile parameter

v2:
  - fixed return code if swtpm_setup doesn't support the option
---
 src/qemu/qemu_tpm.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 src/util/virtpm.c   |  1 +
 src/util/virtpm.h   |  1 +
 3 files changed, 44 insertions(+)

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 100481503c..f797a87fdc 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -385,6 +385,45 @@ 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;
+    g_autofree char *errbuf = 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);
+    virCommandSetErrorBuffer(cmd, &errbuf);
+
+    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not run '%s' to create config files. "
+                         "exitstatus: %d;\nError: %s"),
+                          swtpm_setup, exitstatus, errbuf);
+        return -1;
+    }
+
+    return 0;
+}
+
+
 /*
  * qemuTPMEmulatorRunSetup
  *
@@ -432,6 +471,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 v4] qemu: tpm: Run swtpm_setup --create-config-files in session mode

Posted by Daniel P. Berrangé 3 days, 1 hour ago
On Wed, Oct 13, 2021 at 08:57:13AM -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.
> 
> 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>
> 
> ---
> v4:
>   - Append stderr output to virReportError if swtpm_setup fails
> 
> v3:
>   - Removed logfile parameter
> 
> v2:
>   - fixed return code if swtpm_setup doesn't support the option
> ---
>  src/qemu/qemu_tpm.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  src/util/virtpm.c   |  1 +
>  src/util/virtpm.h   |  1 +
>  3 files changed, 44 insertions(+)
> 
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 100481503c..f797a87fdc 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -385,6 +385,45 @@ 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;
> +    g_autofree char *errbuf = 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);
> +    virCommandSetErrorBuffer(cmd, &errbuf);
> +
> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {

If  virCommandRun returns < 0, it will have already reported an
error message, so you only need to report an error in the exitstatus != 0
case.

> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Could not run '%s' to create config files. "
> +                         "exitstatus: %d;\nError: %s"),
> +                          swtpm_setup, exitstatus, errbuf);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  /*
>   * qemuTPMEmulatorRunSetup
>   *
> @@ -432,6 +471,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>                                   "this requires privileged mode for a "
>                                   "TPM 1.2\n"), 0600);
>  
> +    if (!privileged && qemuTPMCreateConfigFiles())
> +        return -1;

The error return value is < 0, rather than != 0, so the
test should be:

   && qemuTPMCreateConfigFiles() < 0)

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

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 v4] qemu: tpm: Run swtpm_setup --create-config-files in session mode

Posted by Marc-André Lureau 1 week, 1 day ago
Hi

On Wed, Oct 13, 2021 at 4:57 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>
>
> ---
> v4:
>   - Append stderr output to virReportError if swtpm_setup fails
>
> v3:
>   - Removed logfile parameter
>
> v2:
>   - fixed return code if swtpm_setup doesn't support the option
> ---
>  src/qemu/qemu_tpm.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  src/util/virtpm.c   |  1 +
>  src/util/virtpm.h   |  1 +
>  3 files changed, 44 insertions(+)
>
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 100481503c..f797a87fdc 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -385,6 +385,45 @@ 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;
> +    g_autofree char *errbuf = 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);
> +    virCommandSetErrorBuffer(cmd, &errbuf);
> +
> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Could not run '%s' to create config files. "
> +                         "exitstatus: %d;\nError: %s"),
> +                          swtpm_setup, exitstatus, errbuf);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  /*
>   * qemuTPMEmulatorRunSetup
>   *
> @@ -432,6 +471,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
>
>
lgtm,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>