Re: [PATCH v4 1/1] tpm: add sysfs exports for all banks of PCR registers

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Wed, Aug 19, 2020 at 03:48:11PM -0700, Jerry Snitselaar wrote:
> 
> Jarkko Sakkinen @ 2020-08-19 15:16 MST:
> 
> > On Wed, Aug 19, 2020 at 10:53:38AM -0400, Mimi Zohar wrote:
> >> On Wed, 2020-08-19 at 11:09 -0300, Jason Gunthorpe wrote:
> >> > On Wed, Aug 19, 2020 at 09:27:33AM -0400, Mimi Zohar wrote:
> >> > > On Wed, 2020-08-19 at 09:02 -0300, Jason Gunthorpe wrote:
> >> > > > On Tue, Aug 18, 2020 at 02:55:50PM -0400, Mimi Zohar wrote:
> >> > > > 
> >> > > > > The problem is that there isn't just one single userspace library or
> >> > > > > application for reading PCRs.  So now not only is there the kernel
> >> > > > > "boot_aggregate" regression testing, but regression testing of the tool
> >> > > > > itself to support multiple methods of reading the PCRs.
> >> > > > 
> >> > > > I was thinking just open code 
> >> > > >   open("/dev/tpm")
> >> > > >   write(read_pcrs_cmd)
> >> > > >   read(read_pcrs_cmd)
> >> > > >  
> >> > > > It isn't particularly hard to retrive the PCRs, don't really need to
> >> > > > depend on a library.
> >> > > 
> >> > > Ok, do you want to contribute it to ima-evm-utils?  While you're at it,
> >> > > do you also have code to parse the TPM 2.0 event log that you could
> >> > > contribute?
> >> > > 
> >> > > Seriously, we shouldn't be (re-)writing code to do this.
> >> > 
> >> > The kernel should not be used a dumping ground to work around a
> >> > dysfunctional userspace either. :(
> >> > 
> >> > You've basicaly said you can't rely on a sane userspace library
> >> > because *reasons* so we need to dump stuff in the kernel instead.
> >> > 
> >> > It is not a good justification to add new uAPI.
> >> > 
> >> > James seems to have the same basic conclusion too, unfortunately.
> >> 
> >> "dysfunctional" is dropping existing TPM 1.2 sysfs support, which was
> >> done without consideration about existing applications/tools (e.g. ima-
> >> evm-utils, ltp) and without community input.  It's not only James that
> >> is advocating for exporting the TPM PCRs, but Jerry Snitselaar, who
> >> reviewed this patch and exported the TPM version, and Nayna Jain, who
> >> exported the TPM 2.0 event log.  I'm pretty sure there are a number of
> >> other people who would agree.
> >> 
> >> Mimi
> >
> > This is not true. TPM 1.2 sysfs was not dropped.
> >
> > Not adding something does not mean technically dropping something.
> >
> > /Jarkko
> 
> When reviewing it I honestly didn't give much(any?) thought to whether
> it should be there. My thought was it adhered to the 1 value per file
> rule unlike the 1.2 pcrs file and that was about it.
> 
> IIRC when 2.0 was added there was the issue of things like the 1.2 pcrs
> not conforming to standards, possible issues of races, and a question of
> what exactly should be exported.  1.2 has a number of files for doing
> things that I think were also handled by ppi and userspace.
> 
> I guess the question is where does the line get drawn. My exporting the
> major version of the tpm probably could've been handled instead with a
> pr_info spitting it out for people to grab out of dmesg.
> 
> 
> Jerry

TPM protocol version is a different case tha dumping all the PCRs as
ASCII. It fairly unintrusive feature for the kernel, kernel has this
knowledge stored already and it is constant for a boot cycle.

/Jarkko