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

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

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

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

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
participants (2)
-
Marc-André Lureau
-
Stefan Berger