public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [RFC 0/2] Initial TPM support for VMs
@ 2021-07-15 14:23 Stefan Reiter
  2021-07-15 14:23 ` [pve-devel] [RFC edk2-firmware 1/2] enable TPM and TPM2 support Stefan Reiter
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Stefan Reiter @ 2021-07-15 14:23 UTC (permalink / raw)
  To: pve-devel

Makes Windows 11 (test build) happy: https://i.imgur.com/kZ0Mpnr.jpeg

Tested under Linux as well, works with (updated) OVMF and SeaBIOS, though
SeaBIOS requires clearing via the BIOS setup screen and may not support all
features it seems (e.g. Windows shows the TPM, but doesn't allow BitLocker,
presumably because it requires UEFI).

Requires swtpm and libtpms:
https://github.com/stefanberger/swtpm (stable-0.6)
https://github.com/stefanberger/libtpms (stable-0.8)
Both repos allow debian builds, that's what I used for testing.

Decided to send a WIP version before leaving on vacation, so you can take your
time with feedback :)

Design decision: 'swtpm' requires a *directory* per VM to store data, the files
themselves are rather small (8.5kB for an initialized TPM 2.0 w/ BitLocker
enabled). To allow for easier HA I went with a path in '/etc/pve/priv' for now,
but that comes with it's own drawbacks. For example, a guest may write TPM state
as often as they like, and Windows seems to do so every few seconds at random.
(note: swtpm writes a temporary file and then uses atomic replace)

Other ideas for this:
* store in regular path, e.g. '/var/lib' - how to do HA? (note that
  live-migration works regardless, since the state is transferred via QEMU)
* treat like efidisk and allow any storage - would be my favourite, but as said,
  swtpm requires a directory, not a single file...

Missing feature/known issues:
* backups and offline-snapshots don't include TPM data
* migration may hickup, since destination and target are effectively "the same"
* needs improved clearing logic, or potentially none at all (would be a benefit
  for the "efidisk-like" variant)


edk2-firmware: Stefan Reiter (1):
  enable TPM and TPM2 support

 debian/rules | 4 ++++
 1 file changed, 4 insertions(+)

qemu-server: Stefan Reiter (1):
  fix #3075: add TPM v1.2 and v2.0 support via swtpm

 PVE/QemuServer.pm | 134 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 1 deletion(-)

-- 
2.30.2




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [RFC edk2-firmware 1/2] enable TPM and TPM2 support
  2021-07-15 14:23 [pve-devel] [RFC 0/2] Initial TPM support for VMs Stefan Reiter
@ 2021-07-15 14:23 ` Stefan Reiter
  2021-07-15 14:23 ` [pve-devel] [RFC qemu-server 2/2] fix #3075: add TPM v1.2 and v2.0 support via swtpm Stefan Reiter
  2021-07-16  9:48 ` [pve-devel] [RFC 0/2] Initial TPM support for VMs Thomas Lamprecht
  2 siblings, 0 replies; 7+ messages in thread
From: Stefan Reiter @ 2021-07-15 14:23 UTC (permalink / raw)
  To: pve-devel

Necessary for an OS to use a TPM attached to a OVMF VM.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 debian/rules | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/debian/rules b/debian/rules
index fb85b29..ece77be 100755
--- a/debian/rules
+++ b/debian/rules
@@ -54,6 +54,8 @@ build-ovmf: setup-build
 		    -a $(EDK2_HOST_ARCH) \
 		    -t $(EDK2_TOOLCHAIN) \
 		    -DSECURE_BOOT_ENABLE=FALSE \
+		    -DTPM_ENABLE=TRUE \
+		    -DTPM2_ENABLE=TRUE \
 		    -DFD_SIZE_2MB \
 		    -n $$(getconf _NPROCESSORS_ONLN)
 
@@ -75,6 +77,8 @@ build-qemu-efi: setup-build
 			-p ArmVirtPkg/ArmVirtQemu.dsc \
 			-DHTTP_BOOT_ENABLE=TRUE \
 			-DSECURE_BOOT_ENABLE=FALSE \
+			-DTPM_ENABLE=TRUE \
+			-DTPM2_ENABLE=TRUE \
 			-DINTEL_BDS \
 			-b RELEASE
 	dd if=/dev/zero of=Build/ArmVirtQemu-$(EDK2_HOST_ARCH)/RELEASE_$(EDK2_TOOLCHAIN)/FV/$(FW_NAME)_CODE.fd bs=1M seek=64 count=0
-- 
2.30.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [RFC qemu-server 2/2] fix #3075: add TPM v1.2 and v2.0 support via swtpm
  2021-07-15 14:23 [pve-devel] [RFC 0/2] Initial TPM support for VMs Stefan Reiter
  2021-07-15 14:23 ` [pve-devel] [RFC edk2-firmware 1/2] enable TPM and TPM2 support Stefan Reiter
@ 2021-07-15 14:23 ` Stefan Reiter
  2021-07-16 14:47   ` alexandre derumier
  2021-08-09 18:17   ` Nick Chevsky
  2021-07-16  9:48 ` [pve-devel] [RFC 0/2] Initial TPM support for VMs Thomas Lamprecht
  2 siblings, 2 replies; 7+ messages in thread
From: Stefan Reiter @ 2021-07-15 14:23 UTC (permalink / raw)
  To: pve-devel

Starts an instance of swtpm per VM in it's systemd scope, it will
terminate by itself if the VM exits, or be terminated manually if
startup fails.

Before first use, a TPM state is created via swtpm_setup. The state
lives in "/etc/pve/priv/tpm/<vmid>-<version>/".

TPM state is cleared if the 'tpm' config option is removed or the
version changed, effectively clearing any stored keys/data.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 PVE/QemuServer.pm | 134 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b0fe257..76a25ae 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -686,6 +686,12 @@ EODESCR
 	description => "Configure a VirtIO-based Random Number Generator.",
 	optional => 1,
     },
+    tpm => {
+	optional => 1,
+	type => 'string',
+	enum => [ qw(v1.2 v2.0) ],
+	description => "Configure an emulated Trusted Platform Module.",
+    },
 };
 
 my $cicustom_fmt = {
@@ -2945,6 +2951,116 @@ sub audio_devs {
     return $devs;
 }
 
+sub get_tpm_paths {
+    my ($vmid, $version) = @_;
+    return {
+	state => "/etc/pve/priv/tpm/$vmid-$version/",
+	socket => "/var/run/qemu-server/$vmid.swtpm",
+	pid => "/var/run/qemu-server/$vmid.swtpm.pid",
+	filename => $version eq "v1.2" ? "tpm-00.permall" : "tpm2-00.permall",
+    };
+}
+
+sub print_tpm_device {
+    my ($vmid, $version) = @_;
+    my $paths = get_tpm_paths($vmid, $version);
+
+    my $devs = [];
+
+    push @$devs, "-chardev", "socket,id=tpmchar,path=$paths->{socket}";
+    push @$devs, "-tpmdev", "emulator,id=tpmdev,chardev=tpmchar";
+    push @$devs, "-device", "tpm-tis,tpmdev=tpmdev";
+
+    return $devs;
+}
+
+sub start_swtpm {
+    my ($vmid, $version, $migration) = @_;
+    my $paths = get_tpm_paths($vmid, $version);
+
+    if ($migration) {
+	# we will get migration state from remote, so remove any pre-existing
+	clear_tpm_states($vmid);
+	File::Path::make_path($paths->{state});
+    } else {
+	# run swtpm_setup to create a new TPM state if it doesn't exist yet
+	if (! -f "$paths->{state}/$paths->{filename}") {
+	    print "Creating new TPM state\n";
+
+	    # swtpm_setup does not like /etc/pve/priv, so create in tempdir
+	    my $tmppath = "/tmp/tpm-$vmid-$$";
+	    File::Path::make_path($tmppath, mode => 0600);
+	    my $setup_cmd = [
+		"swtpm_setup",
+		"--tpmstate",
+		"$tmppath",
+		"--createek",
+		"--create-ek-cert",
+		"--create-platform-cert",
+		"--lock-nvram",
+		"--config",
+		"/etc/swtpm_setup.conf", # do not use XDG configs
+		"--runas",
+		"0", # force creation as root, error if not possible
+	    ];
+
+	    push @$setup_cmd, "--tpm2" if $version eq 'v2.0';
+	    # TPM 2.0 supports ECC crypto, use if possible
+	    push @$setup_cmd, "--ecc" if $version eq 'v2.0';
+
+	    # produces a lot of verbose output, only show on error
+	    my $tpmout = "";
+	    run_command($setup_cmd, outfunc => sub {
+		$tpmout .= $1 . "\n";
+	    });
+
+	    File::Path::make_path($paths->{state});
+	    my $res = File::Copy::move("$tmppath/$paths->{filename}",
+		"$paths->{state}/$paths->{filename}");
+	    File::Path::rmtree($tmppath);
+	    if (!$res) {
+		my $err = $!;
+		File::Path::rmtree($tmppath);
+		print "swtpm_setup reported:\n$tpmout";
+		die "couldn't move TPM state into '$paths->{state}' - $err\n";
+	    }
+	}
+    }
+
+    my $emulator_cmd = [
+	"swtpm",
+	"socket",
+	"--tpmstate",
+	"dir=$paths->{state},mode=0600",
+	"--ctrl",
+	"type=unixio,path=$paths->{socket},mode=0600",
+	"--pid",
+	"file=$paths->{pid}",
+	"--terminate", # terminate on QEMU disconnect
+	"--daemon",
+    ];
+    push @$emulator_cmd, "--tpm2" if $version eq 'v2.0';
+    run_command($emulator_cmd);
+
+    # return untainted PID of swtpm daemon so it can be killed on error
+    file_read_firstline($paths->{pid}) =~ m/(\d+)/;
+    return $1;
+}
+
+# clear any TPM states other than the ones relevant for $version
+sub clear_tpm_states {
+    my ($vmid, $keep_version) = @_;
+
+    my $clear = sub {
+	my ($v) = @_;
+	my $paths = get_tpm_paths($vmid, $v);
+	rmtree $paths->{state};
+    };
+
+    &$clear("v1.2") if !$keep_version || $keep_version ne "v1.2";
+    &$clear("v2.0") if !$keep_version || $keep_version ne "v2.0";
+}
+
 sub vga_conf_has_spice {
     my ($vga) = @_;
 
@@ -3446,6 +3562,11 @@ sub config_to_command {
 	push @$devices, @$audio_devs;
     }
 
+    if (my $tpmver = $conf->{tpm}) {
+	my $tpmdev = print_tpm_device($vmid, $tpmver);
+	push @$devices, @$tpmdev;
+    }
+
     my $sockets = 1;
     $sockets = $conf->{smp} if $conf->{smp}; # old style - no longer iused
     $sockets = $conf->{sockets} if  $conf->{sockets};
@@ -4829,6 +4950,8 @@ sub vmconfig_apply_pending {
 	}
     }
 
+    PVE::QemuServer::clear_tpm_states($vmid, $conf->{tpm});
+
     # write all changes at once to avoid unnecessary i/o
     PVE::QemuConfig->write_config($vmid, $conf);
 }
@@ -5329,8 +5452,17 @@ sub vm_start_nolock {
 	PVE::Tools::run_fork sub {
 	    PVE::Systemd::enter_systemd_scope($vmid, "Proxmox VE VM $vmid", %properties);
 
+	    my $tpmpid;
+	    if (my $tpmver = $conf->{tpm}) {
+		# start the TPM emulator so QEMU can connect on start
+		$tpmpid = start_swtpm($vmid, $tpmver, $migratedfrom);
+	    }
+
 	    my $exitcode = run_command($cmd, %run_params);
-	    die "QEMU exited with code $exitcode\n" if $exitcode;
+	    if ($exitcode) {
+		kill 'TERM', $tpmpid if $tpmpid;
+		die "QEMU exited with code $exitcode\n";
+	    }
 	};
     };
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [RFC 0/2] Initial TPM support for VMs
  2021-07-15 14:23 [pve-devel] [RFC 0/2] Initial TPM support for VMs Stefan Reiter
  2021-07-15 14:23 ` [pve-devel] [RFC edk2-firmware 1/2] enable TPM and TPM2 support Stefan Reiter
  2021-07-15 14:23 ` [pve-devel] [RFC qemu-server 2/2] fix #3075: add TPM v1.2 and v2.0 support via swtpm Stefan Reiter
@ 2021-07-16  9:48 ` Thomas Lamprecht
  2 siblings, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2021-07-16  9:48 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Reiter

On 15.07.21 16:23, Stefan Reiter wrote:
> Design decision: 'swtpm' requires a *directory* per VM to store data, the files
> themselves are rather small (8.5kB for an initialized TPM 2.0 w/ BitLocker
> enabled). To allow for easier HA I went with a path in '/etc/pve/priv' for now,
> but that comes with it's own drawbacks. For example, a guest may write TPM state
> as often as they like, and Windows seems to do so every few seconds at random.
> (note: swtpm writes a temporary file and then uses atomic replace)
> 
> Other ideas for this:
> * store in regular path, e.g. '/var/lib' - how to do HA? (note that
>   live-migration works regardless, since the state is transferred via QEMU)
> * treat like efidisk and allow any storage - would be my favourite, but as said,
>   swtpm requires a directory, not a single file...
> 
> Missing feature/known issues:
> * backups and offline-snapshots don't include TPM data
> * migration may hickup, since destination and target are effectively "the same"
> * needs improved clearing logic, or potentially none at all (would be a benefit
>   for the "efidisk-like" variant)

For the record, I talked about this with Stefan yesterday before he left for vacation,
and while the size seems pretty much OK for a file on pmxcfs the frequent updates are
just a no go, so that approach is NAK'd (not only be me, Dietmar voiced his objections
too).

I recommended asking swtpm upstream if they'd accept options for passing a explicit
TPM state-file and a separate run-state dir for the swtpm locks and state (we wouldn't
even need the swtmp locks as access synchronization of the state-file would be already
by the pve-storage stack locks, but I do not care much for those as there'd be no
contention anyway).

Then we can put the actual state on a volume similar to EFI disks and the ephemeral
run-state somewhere in "/run/qemu-server/$vmid.swtpm/", with that we got snapshots,
migrations (when local storage is used) and backups covered to rather easily.

If upstream is reluctant then an idea would be to either patch it ourself or use the
libtpms to write a replacement tool in rust, but as said, I'd always try to discuss
our needs with upstream first - IMO other (clustered) hyper-visor systems won't be to
happy as is either.




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [RFC qemu-server 2/2] fix #3075: add TPM v1.2 and v2.0 support via swtpm
  2021-07-15 14:23 ` [pve-devel] [RFC qemu-server 2/2] fix #3075: add TPM v1.2 and v2.0 support via swtpm Stefan Reiter
@ 2021-07-16 14:47   ` alexandre derumier
  2021-08-09 18:17   ` Nick Chevsky
  1 sibling, 0 replies; 7+ messages in thread
From: alexandre derumier @ 2021-07-16 14:47 UTC (permalink / raw)
  To: Proxmox VE development discussion

Hi, I have found old post where a nvram device (using a qcow2 or raw
file as backend) was used with tpm

https://libvir-list.redhat.narkive.com/eOrJPdYX/libvirt-how-libvirt-address-qemu-command-line-args
also dev doc from 2011 mention it

https://wiki.qemu.org/Features/TPM

(I don't have tested to verify)


Le jeudi 15 juillet 2021 à 16:23 +0200, Stefan Reiter a écrit :
> Starts an instance of swtpm per VM in it's systemd scope, it will
> terminate by itself if the VM exits, or be terminated manually if
> startup fails.
> 
> Before first use, a TPM state is created via swtpm_setup. The state
> lives in "/etc/pve/priv/tpm/<vmid>-<version>/".
> 
> TPM state is cleared if the 'tpm' config option is removed or the
> version changed, effectively clearing any stored keys/data.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>  PVE/QemuServer.pm | 134
> +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 133 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index b0fe257..76a25ae 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -686,6 +686,12 @@ EODESCR
>         description => "Configure a VirtIO-based Random Number
> Generator.",
>         optional => 1,
>      },
> +    tpm => {
> +       optional => 1,
> +       type => 'string',
> +       enum => [ qw(v1.2 v2.0) ],
> +       description => "Configure an emulated Trusted Platform
> Module.",
> +    },
>  };
>  
>  my $cicustom_fmt = {
> @@ -2945,6 +2951,116 @@ sub audio_devs {
>      return $devs;
>  }
>  
> +sub get_tpm_paths {
> +    my ($vmid, $version) = @_;
> +    return {
> +       state => "/etc/pve/priv/tpm/$vmid-$version/",
> +       socket => "/var/run/qemu-server/$vmid.swtpm",
> +       pid => "/var/run/qemu-server/$vmid.swtpm.pid",
> +       filename => $version eq "v1.2" ? "tpm-00.permall" : "tpm2-
> 00.permall",
> +    };
> +}
> +
> +sub print_tpm_device {
> +    my ($vmid, $version) = @_;
> +    my $paths = get_tpm_paths($vmid, $version);
> +
> +    my $devs = [];
> +
> +    push @$devs, "-chardev", "socket,id=tpmchar,path=$paths-
> >{socket}";
> +    push @$devs, "-tpmdev", "emulator,id=tpmdev,chardev=tpmchar";
> +    push @$devs, "-device", "tpm-tis,tpmdev=tpmdev";
> +
> +    return $devs;
> +}
> +
> +sub start_swtpm {
> +    my ($vmid, $version, $migration) = @_;
> +    my $paths = get_tpm_paths($vmid, $version);
> +
> +    if ($migration) {
> +       # we will get migration state from remote, so remove any pre-
> existing
> +       clear_tpm_states($vmid);
> +       File::Path::make_path($paths->{state});
> +    } else {
> +       # run swtpm_setup to create a new TPM state if it doesn't
> exist yet
> +       if (! -f "$paths->{state}/$paths->{filename}") {
> +           print "Creating new TPM state\n";
> +
> +           # swtpm_setup does not like /etc/pve/priv, so create in
> tempdir
> +           my $tmppath = "/tmp/tpm-$vmid-$$";
> +           File::Path::make_path($tmppath, mode => 0600);
> +           my $setup_cmd = [
> +               "swtpm_setup",
> +               "--tpmstate",
> +               "$tmppath",
> +               "--createek",
> +               "--create-ek-cert",
> +               "--create-platform-cert",
> +               "--lock-nvram",
> +               "--config",
> +               "/etc/swtpm_setup.conf", # do not use XDG configs
> +               "--runas",
> +               "0", # force creation as root, error if not possible
> +           ];
> +
> +           push @$setup_cmd, "--tpm2" if $version eq 'v2.0';
> +           # TPM 2.0 supports ECC crypto, use if possible
> +           push @$setup_cmd, "--ecc" if $version eq 'v2.0';
> +
> +           # produces a lot of verbose output, only show on error
> +           my $tpmout = "";
> +           run_command($setup_cmd, outfunc => sub {
> +               $tpmout .= $1 . "\n";
> +           });
> +
> +           File::Path::make_path($paths->{state});
> +           my $res = File::Copy::move("$tmppath/$paths->{filename}",
> +               "$paths->{state}/$paths->{filename}");
> +           File::Path::rmtree($tmppath);
> +           if (!$res) {
> +               my $err = $!;
> +               File::Path::rmtree($tmppath);
> +               print "swtpm_setup reported:\n$tpmout";
> +               die "couldn't move TPM state into '$paths->{state}' -
> $err\n";
> +           }
> +       }
> +    }
> +
> +    my $emulator_cmd = [
> +       "swtpm",
> +       "socket",
> +       "--tpmstate",
> +       "dir=$paths->{state},mode=0600",
> +       "--ctrl",
> +       "type=unixio,path=$paths->{socket},mode=0600",
> +       "--pid",
> +       "file=$paths->{pid}",
> +       "--terminate", # terminate on QEMU disconnect
> +       "--daemon",
> +    ];
> +    push @$emulator_cmd, "--tpm2" if $version eq 'v2.0';
> +    run_command($emulator_cmd);
> +
> +    # return untainted PID of swtpm daemon so it can be killed on
> error
> +    file_read_firstline($paths->{pid}) =~ m/(\d+)/;
> +    return $1;
> +}
> +
> +# clear any TPM states other than the ones relevant for $version
> +sub clear_tpm_states {
> +    my ($vmid, $keep_version) = @_;
> +
> +    my $clear = sub {
> +       my ($v) = @_;
> +       my $paths = get_tpm_paths($vmid, $v);
> +       rmtree $paths->{state};
> +    };
> +
> +    &$clear("v1.2") if !$keep_version || $keep_version ne "v1.2";
> +    &$clear("v2.0") if !$keep_version || $keep_version ne "v2.0";
> +}
> +
>  sub vga_conf_has_spice {
>      my ($vga) = @_;
>  
> @@ -3446,6 +3562,11 @@ sub config_to_command {
>         push @$devices, @$audio_devs;
>      }
>  
> +    if (my $tpmver = $conf->{tpm}) {
> +       my $tpmdev = print_tpm_device($vmid, $tpmver);
> +       push @$devices, @$tpmdev;
> +    }
> +
>      my $sockets = 1;
>      $sockets = $conf->{smp} if $conf->{smp}; # old style - no longer
> iused
>      $sockets = $conf->{sockets} if  $conf->{sockets};
> @@ -4829,6 +4950,8 @@ sub vmconfig_apply_pending {
>         }
>      }
>  
> +    PVE::QemuServer::clear_tpm_states($vmid, $conf->{tpm});
> +
>      # write all changes at once to avoid unnecessary i/o
>      PVE::QemuConfig->write_config($vmid, $conf);
>  }
> @@ -5329,8 +5452,17 @@ sub vm_start_nolock {
>         PVE::Tools::run_fork sub {
>             PVE::Systemd::enter_systemd_scope($vmid, "Proxmox VE VM
> $vmid", %properties);
>  
> +           my $tpmpid;
> +           if (my $tpmver = $conf->{tpm}) {
> +               # start the TPM emulator so QEMU can connect on start
> +               $tpmpid = start_swtpm($vmid, $tpmver, $migratedfrom);
> +           }
> +
>             my $exitcode = run_command($cmd, %run_params);
> -           die "QEMU exited with code $exitcode\n" if $exitcode;
> +           if ($exitcode) {
> +               kill 'TERM', $tpmpid if $tpmpid;
> +               die "QEMU exited with code $exitcode\n";
> +           }
>         };
>      };
>  






^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [RFC qemu-server 2/2] fix #3075: add TPM v1.2 and v2.0 support via swtpm
  2021-07-15 14:23 ` [pve-devel] [RFC qemu-server 2/2] fix #3075: add TPM v1.2 and v2.0 support via swtpm Stefan Reiter
  2021-07-16 14:47   ` alexandre derumier
@ 2021-08-09 18:17   ` Nick Chevsky
  2021-08-10  7:48     ` Stefan Reiter
  1 sibling, 1 reply; 7+ messages in thread
From: Nick Chevsky @ 2021-08-09 18:17 UTC (permalink / raw)
  To: Proxmox VE development discussion

Hi Stefan,

Thank you for your work on this; I've been testing it locally for a few
weeks and have since contributed improved Debian packaging and other fixes
upstream [3]. Please see my comment below the quoted code:

--- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> ...
> +sub start_swtpm {
> ...
> +           my $setup_cmd = [
> +               "swtpm_setup",
> +               "--tpmstate",
> +               "$tmppath",
> +               "--createek",
> +               "--create-ek-cert",
> +               "--create-platform-cert",
> +               "--lock-nvram",
> +               "--config",
> +               "/etc/swtpm_setup.conf", # do not use XDG configs
> +               "--runas",
> +               "0", # force creation as root, error if not possible
>

Could you add --terminate to this argument array? That's the documented,
correct way of achieving the behavior we want (i.e. swtpm automatically
terminating along with QEMU). Currently this is already happening even
without --terminate, but that's a side effect of two bugs: one for which
I've already contributed a fix upstream [1], and another which will be
fixed once consumers (e.g. PVE, libvirt) start using --terminate (which
they should've been using all along) [2]. Adding --terminate is innocuous
and guarantees the current behavior will stay the same after the second bug
is fixed upstream.

[1]
https://github.com/stefanberger/swtpm/commit/6961ec4878b4a569ac53f6e6f77416b44f3f26d9
[2] https://github.com/stefanberger/swtpm/pull/509#issuecomment-890412478
[3] https://github.com/stefanberger/swtpm/pulls?q=author%3Anchevsky

Nick


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [RFC qemu-server 2/2] fix #3075: add TPM v1.2 and v2.0 support via swtpm
  2021-08-09 18:17   ` Nick Chevsky
@ 2021-08-10  7:48     ` Stefan Reiter
  0 siblings, 0 replies; 7+ messages in thread
From: Stefan Reiter @ 2021-08-10  7:48 UTC (permalink / raw)
  To: Proxmox VE development discussion, Nick Chevsky

On 09/08/2021 20:17, Nick Chevsky wrote:
> Hi Stefan,
> 
> Thank you for your work on this; I've been testing it locally for a few
> weeks and have since contributed improved Debian packaging and other fixes
> upstream [3]. Please see my comment below the quoted code:

Thanks for your upstream work! I've seen some of those changes already, 
they'll certainly help a lot.

As a status update to this in general, since we decided that storing 
data on /etc/pve is a no-go and don't have a generic dir backend for all 
use-cases we want to support, I have also started working with upstream 
(based on some prior work being done in a current draft PR [0]) to 
support block devices as a native backend [1].

With that in place, we should be able to use our existing storage 
infrastructure for storing TPM state.

[0] https://github.com/stefanberger/swtpm/pull/490
[1] https://github.com/stefanberger/swtpm/pull/513

> 
> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> ...
>> +sub start_swtpm {
>> ...
>> +           my $setup_cmd = [
>> +               "swtpm_setup",
>> +               "--tpmstate",
>> +               "$tmppath",
>> +               "--createek",
>> +               "--create-ek-cert",
>> +               "--create-platform-cert",
>> +               "--lock-nvram",
>> +               "--config",
>> +               "/etc/swtpm_setup.conf", # do not use XDG configs
>> +               "--runas",
>> +               "0", # force creation as root, error if not possible
>>
> 
> Could you add --terminate to this argument array? That's the documented,
> correct way of achieving the behavior we want (i.e. swtpm automatically
> terminating along with QEMU). Currently this is already happening even
> without --terminate, but that's a side effect of two bugs: one for which
> I've already contributed a fix upstream [1], and another which will be
> fixed once consumers (e.g. PVE, libvirt) start using --terminate (which
> they should've been using all along) [2]. Adding --terminate is innocuous
> and guarantees the current behavior will stay the same after the second bug
> is fixed upstream.

Good to hear the explanation behind it, but '--terminate' is already 
added - you're just looking at the 'swtpm_setup' command, check the 
lines below that, specifically '$emulator_cmd'.

> 
> [1]
> https://github.com/stefanberger/swtpm/commit/6961ec4878b4a569ac53f6e6f77416b44f3f26d9
> [2] https://github.com/stefanberger/swtpm/pull/509#issuecomment-890412478
> [3] https://github.com/stefanberger/swtpm/pulls?q=author%3Anchevsky
> 
> Nick
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-08-10  7:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 14:23 [pve-devel] [RFC 0/2] Initial TPM support for VMs Stefan Reiter
2021-07-15 14:23 ` [pve-devel] [RFC edk2-firmware 1/2] enable TPM and TPM2 support Stefan Reiter
2021-07-15 14:23 ` [pve-devel] [RFC qemu-server 2/2] fix #3075: add TPM v1.2 and v2.0 support via swtpm Stefan Reiter
2021-07-16 14:47   ` alexandre derumier
2021-08-09 18:17   ` Nick Chevsky
2021-08-10  7:48     ` Stefan Reiter
2021-07-16  9:48 ` [pve-devel] [RFC 0/2] Initial TPM support for VMs Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal