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:46:15PM -0700, James Bottomley wrote:
> On Thu, 2020-08-20 at 00:53 +0300, Jarkko Sakkinen wrote:
> > > On Tue, 2020-08-18 at 14:17 -0300, Jason Gunthorpe wrote:
> > > > On Tue, Aug 18, 2020 at 09:44:30AM -0700, James Bottomley wrote:
> > > > 
> > > > > The question you should be asking isn't whether the information
> > > > > *could* be obtained by other means, but whether providing it in
> > > > > this form facilitates current operations and whether the
> > > > > interface would have users.
> > > > 
> > > > Sure. What are the use cases that need PCRs but no other TPM
> > > > operations?
> > > > 
> > > > The cover letter didn't say. As PCR is really only useful in the
> > > > context of the local TPM I'm not thinking of anything..
> > > 
> > > The three use cases I picked up at the Boot and Security MC were:
> > > 
> > >    1. local log verification: a script can run through the IMA
> > > ascii log
> > >       and construct the PCR 10 hash which can then be verified
> > >    2. Knowing what the PCR values actually are for sealed
> > > keys.  With the
> > >       current trusted key infrastructure you have to calculate and
> > > supply
> > >       the hash yourself.  With the new proposed infrastructure, the
> > > hash
> > >       would be calculated by the seal operation, but you're still
> > > going to
> > >       need the actual PCR values to debug unseal failures.
> > >    3. As a stability check for log shipping: you read the PCR take
> > > the log
> > >       then read the PCR: if the two reads are the same the PCR
> > > backing the
> > >       log is stable for quoting.
> > > 
> > > James
> > 
> > The proposed sysfs attributes are racy in the sense that PCRs could
> > change in-between reading different hashes.
> 
> That's not really a problem, is it?  For use case 2. we expect them to
> be stable otherwise you're doing the wrong thing sealing to them. For
> the IMA PCR you use the stability protocol in 3.
> 
> > A blob containing all the hashes would make more sense as it does not
> > have this issue.
> 
> It doesn't really buy anything though.  If you're verifying the log you
> always have the problem that the PCR and the log are at different
> points, so you follow the protocol in 3. or read PCR then log and
> unwind the log until it matches or you've gone too far.
> 
> > If this is for scripts to further process, it is also more efficient
> > than printable ASCII text.
> 
> I'm not fundamentally opposed to binary attributes, but realistically
> if I want the hash of PCRs 1 4 and 6 it's not fundamentally different
> to me whether I do
> 
> cat /sys/class/tpm/tpm0/pcr-sha256/1 /sys/class/tpm/tpm0/pcr-sha256/4 /sys/class/tpm/tpm0/pcr-sha256/6|sha256sum
> 
> or
> 
> cat /sys/class/tpm/tpm0/pcr-sha256/1 /sys/class/tpm/tpm0/pcr-sha256/4 /sys/class/tpm/tpm0/pcr-sha256/6|xxd -r -p|sha256sum
> 
> The point being the tool to convert the hex output back to binary
> already exists and is well known ... and binary attributes have nasty
> console properties if you accidentally cat them directly.
> 
> James

This does not look like a kind of framework of things that we want
to maintain. Especially given that it is easy to get all the data
through /dev/tpm0 easily. It is an enormous addition to uapi with
a questionable value.

/Jarkko