On Tue, Aug 18, 2020 at 02:55:50PM -0400, Mimi Zohar wrote: > 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 It's not hard to write a program to read PCRs with just libc. Takes about one afternoon. /Jarkko
