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

[Jerry Snitselaar <jsnitsel@redhat.com>]

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