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