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

2021-10-08 Thread Stefan Berger 



On 10/8/21 10:56 AM, Stefan Berger wrote:


On 10/8/21 10:52 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.

This patch addresses the issue raised in
https://bugzilla.redhat.com/show_bug.cgi?id=2010649

BTW, I notice the this tool creates certs under $HOME/.config/var
with an expiry date of +10 years.

Now that sounds like a long time, and indeed it is a long time,
but then I look at the support lifetime of RHEL... Hopefully
bare metal hardware won't last for the whole 10 years without
being replaced, but with nested virt the "hosts" could be VMs
that get moved to new hardware.

So what's the story if a host hits the 10 year mark for the
swtpm certs ? Presumably swtpm is validating these dates
and will refuse to launch the TPM for the VMs on the host ?

It doesn't.


I am switching to non-expiring certificates now which should help 
address this issue for future CAs.


These CAs 'created on the fly' were thought of merely as a convenience 
for the user and someone more serious about the TPM CAs would create 
them on their own and use appropriate dates for the expiration and 
manage these certificates before they expire. In a larger setting all 
hosts should share a fairly well-known TPM CA so that all vTPMs' 
certificates are signed with the same CA and certificate validators 
don't need to have n hosts' certs but just '1'. However, that requires 
setup by an admin rather than relying on CAs 'created on the fly'.


Thanks for the feedback

    Stefan





Regards,
Daniel

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

2021-10-08 Thread Stefan Berger 



On 10/8/21 10:52 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.

This patch addresses the issue raised in
https://bugzilla.redhat.com/show_bug.cgi?id=2010649

BTW, I notice the this tool creates certs under $HOME/.config/var
with an expiry date of +10 years.

Now that sounds like a long time, and indeed it is a long time,
but then I look at the support lifetime of RHEL... Hopefully
bare metal hardware won't last for the whole 10 years without
being replaced, but with nested virt the "hosts" could be VMs
that get moved to new hardware.

So what's the story if a host hits the 10 year mark for the
swtpm certs ? Presumably swtpm is validating these dates
and will refuse to launch the TPM for the VMs on the host ?

It doesn't.



Regards,
Daniel

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

2021-10-08 Thread Daniel P . Berrangé 
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 
> > > 
> > > 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_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, ) < 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

2021-10-08 Thread Stefan Berger 



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

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

2021-10-08 Thread Daniel P . Berrangé 
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.
> 
> This patch addresses the issue raised in
> https://bugzilla.redhat.com/show_bug.cgi?id=2010649

BTW, I notice the this tool creates certs under $HOME/.config/var
with an expiry date of +10 years.

Now that sounds like a long time, and indeed it is a long time,
but then I look at the support lifetime of RHEL... Hopefully
bare metal hardware won't last for the whole 10 years without
being replaced, but with nested virt the "hosts" could be VMs
that get moved to new hardware.

So what's the story if a host hits the 10 year mark for the
swtpm certs ? Presumably swtpm is validating these dates
and will refuse to launch the TPM for the VMs on the host ?


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

2021-10-08 Thread Stefan Berger 



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 

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_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, ) < 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

2021-10-08 Thread Daniel P . Berrangé 
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 
> 
> 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_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, ) < 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

2021-10-08 Thread Marc-André Lureau 
On Fri, Oct 8, 2021 at 5:56 PM 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 
>

> 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_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, ) < 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

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

2021-10-08 Thread Stefan Berger 
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 

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_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, ) < 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