On Tue, 2020-08-18 at 15:36 -0300, Jason Gunthorpe wrote: > On Tue, Aug 18, 2020 at 02:26:04PM -0400, Mimi Zohar wrote: > > On Tue, 2020-08-18 at 13:46 -0300, Jason Gunthorpe wrote: > > > On Tue, Aug 18, 2020 at 07:26:30PM +0300, Jarkko Sakkinen wrote: > > > > On Tue, Aug 18, 2020 at 07:19:57PM +0300, Jarkko Sakkinen wrote: > > > > > On Tue, Aug 18, 2020 at 07:12:09PM +0300, Jarkko Sakkinen wrote: > > > > > > On Mon, Aug 17, 2020 at 02:35:06PM -0700, James Bottomley wrote: > > > > > > > Create sysfs per hash groups with 24 PCR files in them one group, > > > > > > > named pcr-<hash>, for each agile hash of the TPM. The files are > > > > > > > plugged in to a PCR read function which is TPM version agnostic, so > > > > > > > this works also for TPM 1.2 but the hash is only sha1 in that case. > > > > > > > > > > > > > > Note: the macros used to create the hashes emit spurious checkpatch > > > > > > > warnings. Do not try to "fix" them as checkpatch recommends, otherwise > > > > > > > they'll break. > > > > > > > > > > > > > > Signed-off-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> > > > > > > > Reviewed-by: Jerry Snitselaar <jsnitsel@xxxxxxxxxx> > > > > > > > Tested-by: Thiago Jung Bauermann <bauerman@xxxxxxxxxxxxx> > > > > > > > > > > > > I have hard time understanding why this is required. > > > > > > > > > > > > You can grab the information through /dev/tpm0 just fine. > > > > > > > > > > I just think it is principally wrong to add sysfs files if they don't > > > > > have any measurable value other than perhaps some convenience. > > > > > > > > > > It is trival to write only a libc dependent program that outputs PCRs. > > > > > > > > > > I think this is essentially an user space problem that is getting sorted > > > > > out with kernel code. > > > > > > > > Jason, what do you make of this? I recall that it was you who I > > > > discussed with about this topic when TPM 2.0 support was first > > > > upstreamed. > > > > > > TPM 2.0 broke all the userspace so it made sense to get rid of the > > > non-conforming sysfs files from TPM v1.x time as part of the userspace > > > API. That was the main reason to not continue forward with PCR in > > > userspace. > > > > > > As far as doing it properly as this patch does.. I agree with you that > > > sysfs files should have some reason to be added, espcially if it > > > causes quite big code cost as this does. eg to drive a udev rule > > > decision. > > > > > > Why is PCRs so special it needs to be in sysfs? What is userspace > > > going to do with this information? > > > > The original IMA LTP "boot_aggregate" regression test is dependent on > > the exported TPM event log and PCRs. Similar support is needed for TPM > > 2.0. There isn't just a single userspace application for reading > > PCRs. As soon as we add support for one userspace > > application, support for the other applications is requested. So > > instead of a having a simple regression test with a single method of > > reading PCRs, we're now required to support multiple userspace > > applications. > > But this test already has a C program as part of the boot aggregate > test, why is it such a problem to use a C program to also read the > PCRs? > > As Jarkko says it is not so hard 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. Mimi
