Re: [libvirt] [PATCH 2/6] tpm: Add support for external swtpm TPM emulator

On 04/06/2018 10:12 AM, Daniel P. Berrangé wrote: > On Fri, Apr 06, 2018 at 07:23:49AM -0400, Stefan Berger wrote: > > On 04/06/2018 04:26 AM, Daniel P. Berrangé wrote: > > > On Thu, Apr 05, 2018 at 05:56:02PM -0400, Stefan Berger wrote: > > > > This patch adds support for an external swtpm TPM emulator. The XML for > > > > this type of TPM looks as follows: > > > > > > > > <tpm model='tpm-tis'> > > > > <backend type='emulator'/> > > > > </tpm> > > > > > > > + cmd = virCommandNew(swtpm_path); > > > > + if (!cmd) > > > > + goto error; > > > > + > > > > + virCommandClearCaps(cmd); > > > > + > > > > + virCommandAddArgList(cmd, "socket", "--daemon", "--ctrl", NULL); > > > > + virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600", > > > > + tpm->data.emulator.source.data.nix.path); > > > > + > > > > + virCommandAddArg(cmd, "--tpmstate"); > > > > + virCommandAddArgFormat(cmd, "dir=%s", storagepath); > > > > + > > > > + virCommandAddArg(cmd, "--log"); > > > > + virCommandAddArgFormat(cmd, "file=%s", logfile); > > > > + > > > > + /* allow process to open logfile by root before dropping privileges */ > > > > + virCommandAllowCap(cmd, CAP_DAC_OVERRIDE); > > > Why can't we get have the log file be owned by the user that > > > swtpm runs as, instead of root ? > > I would have to look at this particular capability again. I initially wanted > > to put the swtpm's log file also into /var/log/libvirt/qemu. It works nice > > of course when running swtpm as 'root' but not so much when running it as > > 'tss': > > > > root@localhost tmp]$ sudo ls -l /var/log/libvirt/ | grep qemu > > drwx------. 2 root root 20480 Apr 5 16:14 qemu > Yeah the logs are owned by root these days, because they're not written by > qemu itself, instead virtlogd owns it. [root@localhost log]# ls -lZ | grep libvirt drwx------. 6 root root system_u:object_r:virt_log_t:s0 4096 Mar 1 2017 libvirt Even /var/log/libvirt would not be accessible for the tss users. > > > So where do we put the swtpm's log files? /var/log/libvirt/swtpm? > Yeah, probably best to have a separate directory It would have to be /var/log/swtpm unless we change the permissions on /var/log/libvirt ... ?

> > > I would feel better if we just directly killed the process - with > > > this approach if something goes wrong with swtpm it may never > > > respond to this request and stay running. > > swtpm can write a pidfile. I am only adding this later in this series. > > Problem is with --daemon libvirt doesn't know the pid of the swtpm anymore. > The other option is to not use --daemon, and let libvirt write the pid > file, but that introduces the race with socket path creation again > which is not good. Sounds like we should leave this as it is? Unless swtpm was broken, there shouldn't be a reason why the we wouldn't be able to shut down swtpm by sending a command to it. The socket and its directory must not have disappeared of course.