[PATCH] ima: skip verifying TPM 2.0 PCR values

[PATCH] ima: skip verifying TPM 2.0 PCR values

[Mimi Zohar]

TPM 1.2 exported the PCRs.  Reading the TPM 2.0 PCRs requires a
userspace application.  For now, skip this test.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 testcases/kernel/security/integrity/ima/tests/ima_tpm.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index 0ffc3c02247d..ebe4b4c360e4 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -88,6 +88,14 @@ test2()
 	tst_res TINFO "verify PCR values"
 	tst_check_cmds evmctl
 
+	local tpm_description="/sys/class/tpm/tpm0/device/description"
+	if [ -f "$tpm_description" ]; then
+		if grep -q "^\TPM 2.0" $tpm_description; then
+			tst_res TCONF "TPM 2.0 enabled, but not supported"
+			return 0
+		fi
+	fi
+
 	tst_res TINFO "evmctl version: $(evmctl --version)"
 
 	local pcrs_path="/sys/class/tpm/tpm0/device/pcrs"
-- 
2.7.5

Re: [PATCH] ima: skip verifying TPM 2.0 PCR values

[Petr Vorel]

Hi Mimi,

> TPM 1.2 exported the PCRs.  Reading the TPM 2.0 PCRs requires a
> userspace application.  For now, skip this test.

> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Acked-by: Petr Vorel <pvorel@suse.cz>
> ---
>  testcases/kernel/security/integrity/ima/tests/ima_tpm.sh | 8 ++++++++
>  1 file changed, 8 insertions(+)

> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> index 0ffc3c02247d..ebe4b4c360e4 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> @@ -88,6 +88,14 @@ test2()
>  	tst_res TINFO "verify PCR values"
>  	tst_check_cmds evmctl

> +	local tpm_description="/sys/class/tpm/tpm0/device/description"
> +	if [ -f "$tpm_description" ]; then
> +		if grep -q "^\TPM 2.0" $tpm_description; then
I guess the backslash in "^\TPM 2.0" is a typo.
If yes, no need to repost, I'll fix it when applying your patch.
+ I'd prefer join 2 ifs into single one, but that's just matter of preference,
not important.

> +			tst_res TCONF "TPM 2.0 enabled, but not supported"
> +			return 0
> +		fi
> +	fi
> +
>  	tst_res TINFO "evmctl version: $(evmctl --version)"

>  	local pcrs_path="/sys/class/tpm/tpm0/device/pcrs"

Thanks for your fix.

Kind regards,
Petr

Re: [PATCH] ima: skip verifying TPM 2.0 PCR values

[Mimi Zohar]

On Fri, 2019-05-17 at 08:51 +0200, Petr Vorel wrote:

> > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > index 0ffc3c02247d..ebe4b4c360e4 100755
> > --- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > @@ -88,6 +88,14 @@ test2()
> >  	tst_res TINFO "verify PCR values"
> >  	tst_check_cmds evmctl
> 
> > +	local tpm_description="/sys/class/tpm/tpm0/device/description"
> > +	if [ -f "$tpm_description" ]; then
> > +		if grep -q "^\TPM 2.0" $tpm_description; then

> I guess the backslash in "^\TPM 2.0" is a typo.
> If yes, no need to repost, I'll fix it when applying your patch.
> + I'd prefer join 2 ifs into single one, but that's just matter of preference,
> not important.

Thank you for fixing it.  I'd just like to hear from others first, if
this is correct way to differentiate between TPM 1.2 and TPM 2.0.

Mimi


> > +			tst_res TCONF "TPM 2.0 enabled, but not supported"
> > +			return 0
> > +		fi
> > +	fi
> > +
> >  	tst_res TINFO "evmctl version: $(evmctl --version)"
> 
> >  	local pcrs_path="/sys/class/tpm/tpm0/device/pcrs"
> 

Re: [PATCH] ima: skip verifying TPM 2.0 PCR values

[Petr Vorel]

Hi Mimi,

> On Fri, 2019-05-17 at 08:51 +0200, Petr Vorel wrote:

> > > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > > index 0ffc3c02247d..ebe4b4c360e4 100755
> > > --- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > > @@ -88,6 +88,14 @@ test2()
> > >  	tst_res TINFO "verify PCR values"
> > >  	tst_check_cmds evmctl

> > > +	local tpm_description="/sys/class/tpm/tpm0/device/description"
> > > +	if [ -f "$tpm_description" ]; then
> > > +		if grep -q "^\TPM 2.0" $tpm_description; then

> > I guess the backslash in "^\TPM 2.0" is a typo.
> > If yes, no need to repost, I'll fix it when applying your patch.
> > + I'd prefer join 2 ifs into single one, but that's just matter of preference,
> > not important.

> Thank you for fixing it.  I'd just like to hear from others first, if
> this is correct way to differentiate between TPM 1.2 and TPM 2.0.
Oh, yes, let's wait for a feedback.

> Mimi

Kind regards,
Petr

Re: [PATCH] ima: skip verifying TPM 2.0 PCR values

[Nayna]

On 05/16/2019 05:12 PM, Mimi Zohar wrote:
> TPM 1.2 exported the PCRs.  Reading the TPM 2.0 PCRs requires a
> userspace application.  For now, skip this test.
>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>   testcases/kernel/security/integrity/ima/tests/ima_tpm.sh | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> index 0ffc3c02247d..ebe4b4c360e4 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> @@ -88,6 +88,14 @@ test2()
>   	tst_res TINFO "verify PCR values"
>   	tst_check_cmds evmctl
>
> +	local tpm_description="/sys/class/tpm/tpm0/device/description"


I do not see a "description" file on either my PowerPC or x86 systems 
with TPM 2.0.  Perhaps instead of testing for the "description" file, if 
the "pcrs" file is not found, emit a more verbose informational message, 
for eg. - "pcrs file is not found - either you are running a TPM 2.0, or 
having sysfs failed to show pcrs for TPM 1.2"

Thanks & Regards,
       - Nayna

Re: [PATCH] ima: skip verifying TPM 2.0 PCR values

[Petr Vorel]

Hi Nayna,

...
> > +	local tpm_description="/sys/class/tpm/tpm0/device/description"
...

> I do not see a "description" file on either my PowerPC or x86 systems with
> TPM 2.0.  Perhaps instead of testing for the "description" file, if the
> "pcrs" file is not found, emit a more verbose informational message, for eg.
> - "pcrs file is not found - either you are running a TPM 2.0, or having
> sysfs failed to show pcrs for TPM 1.2"
Some people are using /sys/class/tpm/tpm0/device/description [1] for testing TPM
version. From the discussion on [1] I also got an expression that the file is
not always presented. If there is really no reliable way to detect TPM version
from sysfs (huh!) your approach would make sense for me.

> Thanks & Regards,
>       - Nayna

Kind regards,
Petr

[1] https://github.com/tpm2-software/tpm2-tools/issues/604

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Petr Vorel]

Hi all,

I wonder what to do with this patch "ima: skip verifying TPM 2.0 PCR values" [1].
Is it a correct way to differentiate between TPM 1.2 and TPM 2.0?
Or something else should be applied?

How is the work on TPM 2.0 Linux sysfs interface?
But even it's done in near future, we'd still need some way for older kernels.

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/patch/1100733/

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Jarkko Sakkinen]

On Thu, Oct 24, 2019 at 02:18:48PM +0200, Petr Vorel wrote:
> Hi all,
> 
> I wonder what to do with this patch "ima: skip verifying TPM 2.0 PCR values" [1].
> Is it a correct way to differentiate between TPM 1.2 and TPM 2.0?
> Or something else should be applied?
> 
> How is the work on TPM 2.0 Linux sysfs interface?
> But even it's done in near future, we'd still need some way for older kernels.
> 
> Kind regards,
> Petr
> 
> [1] https://patchwork.ozlabs.org/patch/1100733/

version_major sysfs file would be acceptable if someone wants to proceed
and send such patch.

Also replicants for durations and timeouts files would make sense for
TPM 2.0.

/Jarkko

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Jason Gunthorpe]

On Thu, Oct 24, 2019 at 08:20:23PM +0300, Jarkko Sakkinen wrote:
> Also replicants for durations and timeouts files would make sense for
> TPM 2.0.

These ones don't meet the sysfs standard of one value per file, which
is why they didn't make it to tpm2

Jason

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Jarkko Sakkinen]

On Thu, Oct 24, 2019 at 03:20:05PM -0300, Jason Gunthorpe wrote:
> On Thu, Oct 24, 2019 at 08:20:23PM +0300, Jarkko Sakkinen wrote:
> > Also replicants for durations and timeouts files would make sense for
> > TPM 2.0.
> 
> These ones don't meet the sysfs standard of one value per file, which
> is why they didn't make it to tpm2

They would be still useful to have available in some form as there is
no way deduce them from the user space.

I'd prioritize in this particular case the compatibility with the legacy
files over sysfs standard with a clear note in the commit message.

/Jarkko

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Jerry Snitselaar]

On Thu Oct 24 19, Jarkko Sakkinen wrote:
>On Thu, Oct 24, 2019 at 02:18:48PM +0200, Petr Vorel wrote:
>> Hi all,
>>
>> I wonder what to do with this patch "ima: skip verifying TPM 2.0 PCR values" [1].
>> Is it a correct way to differentiate between TPM 1.2 and TPM 2.0?
>> Or something else should be applied?
>>
>> How is the work on TPM 2.0 Linux sysfs interface?
>> But even it's done in near future, we'd still need some way for older kernels.
>>
>> Kind regards,
>> Petr
>>
>> [1] https://patchwork.ozlabs.org/patch/1100733/
>
>version_major sysfs file would be acceptable if someone wants to proceed
>and send such patch.
>
>Also replicants for durations and timeouts files would make sense for
>TPM 2.0.
>
>/Jarkko

Is it as simple as doing this?

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index edfa89160010..fd8eb8d8945c 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -309,7 +309,17 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(timeouts);
 
-static struct attribute *tpm_dev_attrs[] = {
+static ssize_t version_major_show(struct device *dev,
+                                 struct device_attribute *attr, char *buf)
+{
+       struct tpm_chip *chip = to_tpm_chip(dev);
+
+       return sprintf(buf, "TPM%s\n", chip->flags & TPM_CHIP_FLAG_TPM2
+                      ? "2.0" : "1.2");
+}
+static DEVICE_ATTR_RO(version_major);
+
+static struct attribute *tpm12_dev_attrs[] = {
        &dev_attr_pubek.attr,
        &dev_attr_pcrs.attr,
        &dev_attr_enabled.attr,
@@ -320,18 +330,28 @@ static struct attribute *tpm_dev_attrs[] = {
        &dev_attr_cancel.attr,
        &dev_attr_durations.attr,
        &dev_attr_timeouts.attr,
+       &dev_attr_version_major.attr,
        NULL,
 };
 
-static const struct attribute_group tpm_dev_group = {
-       .attrs = tpm_dev_attrs,
+static struct attribute *tpm20_dev_attrs[] = {
+       &dev_attr_version_major.attr,
+       NULL
+};
+
+static const struct attribute_group tpm12_dev_group = {
+       .attrs = tpm12_dev_attrs,
+};
+
+static const struct attribute_group tpm20_dev_group = {
+       .attrs = tpm20_dev_attrs,
 };
 
 void tpm_sysfs_add_device(struct tpm_chip *chip)
 {
-       if (chip->flags & TPM_CHIP_FLAG_TPM2)
-               return;
-
        WARN_ON(chip->groups_cnt != 0);
-       chip->groups[chip->groups_cnt++] = &tpm_dev_group;
+       if (chip->flags & TPM_CHIP_FLAG_TPM2)
+               chip->groups[chip->groups_cnt++] = &tpm20_dev_group;
+       else
+               chip->groups[chip->groups_cnt++] = &tpm12_dev_group;
 }


Did a quick test on 2 systems here.

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Jason Gunthorpe]

On Thu, Oct 24, 2019 at 02:38:42PM -0700, Jerry Snitselaar wrote:
> On Thu Oct 24 19, Jarkko Sakkinen wrote:
> > On Thu, Oct 24, 2019 at 02:18:48PM +0200, Petr Vorel wrote:
> > > Hi all,
> > > 
> > > I wonder what to do with this patch "ima: skip verifying TPM 2.0 PCR values" [1].
> > > Is it a correct way to differentiate between TPM 1.2 and TPM 2.0?
> > > Or something else should be applied?
> > > 
> > > How is the work on TPM 2.0 Linux sysfs interface?
> > > But even it's done in near future, we'd still need some way for older kernels.
> > > 
> > > Kind regards,
> > > Petr
> > > 
> > > [1] https://patchwork.ozlabs.org/patch/1100733/
> > 
> > version_major sysfs file would be acceptable if someone wants to proceed
> > and send such patch.
> > 
> > Also replicants for durations and timeouts files would make sense for
> > TPM 2.0.
> > 
> > /Jarkko
> 
> Is it as simple as doing this?
> 
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index edfa89160010..fd8eb8d8945c 100644
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -309,7 +309,17 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_RO(timeouts);
> 
> -static struct attribute *tpm_dev_attrs[] = {
> +static ssize_t version_major_show(struct device *dev,
> +                                 struct device_attribute *attr, char *buf)
> +{
> +       struct tpm_chip *chip = to_tpm_chip(dev);

> +       return sprintf(buf, "TPM%s\n", chip->flags & TPM_CHIP_FLAG_TPM2
> +                      ? "2.0" : "1.2");

Probably no TPM prefix here

The usual sysfs naming would be more like 'major_version'

Jason

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Jason Gunthorpe]

On Thu, Oct 24, 2019 at 10:14:02PM +0300, Jarkko Sakkinen wrote:
> On Thu, Oct 24, 2019 at 03:20:05PM -0300, Jason Gunthorpe wrote:
> > On Thu, Oct 24, 2019 at 08:20:23PM +0300, Jarkko Sakkinen wrote:
> > > Also replicants for durations and timeouts files would make sense for
> > > TPM 2.0.
> > 
> > These ones don't meet the sysfs standard of one value per file, which
> > is why they didn't make it to tpm2
> 
> They would be still useful to have available in some form as there is
> no way deduce them from the user space.

Why? Userspace doesn't refer to these values since the kernel handles
all the timeouts, right?

Jason

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Mimi Zohar]

On Thu, 2019-10-24 at 14:38 -0700, Jerry Snitselaar wrote:
> On Thu Oct 24 19, Jarkko Sakkinen wrote:
> >On Thu, Oct 24, 2019 at 02:18:48PM +0200, Petr Vorel wrote:
> >> Hi all,
> >>
> >> I wonder what to do with this patch "ima: skip verifying TPM 2.0 PCR values" [1].
> >> Is it a correct way to differentiate between TPM 1.2 and TPM 2.0?
> >> Or something else should be applied?
> >>
> >> How is the work on TPM 2.0 Linux sysfs interface?
> >> But even it's done in near future, we'd still need some way for older kernels.
> >>
> >> Kind regards,
> >> Petr
> >>
> >> [1] https://patchwork.ozlabs.org/patch/1100733/
> >
> >version_major sysfs file would be acceptable if someone wants to proceed
> >and send such patch.
> >
> >Also replicants for durations and timeouts files would make sense for
> >TPM 2.0.
> >
> >/Jarkko
> 
> Is it as simple as doing this?
> 
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index edfa89160010..fd8eb8d8945c 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -309,7 +309,17 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(timeouts);
>  
> -static struct attribute *tpm_dev_attrs[] = {
> +static ssize_t version_major_show(struct device *dev,
> +                                 struct device_attribute *attr, char *buf)
> +{
> +       struct tpm_chip *chip = to_tpm_chip(dev);
> +
> +       return sprintf(buf, "TPM%s\n", chip->flags & TPM_CHIP_FLAG_TPM2
> +                      ? "2.0" : "1.2");
> +}
> +static DEVICE_ATTR_RO(version_major);
> +
> +static struct attribute *tpm12_dev_attrs[] = {
>         &dev_attr_pubek.attr,
>         &dev_attr_pcrs.attr,
>         &dev_attr_enabled.attr,
> @@ -320,18 +330,28 @@ static struct attribute *tpm_dev_attrs[] = {
>         &dev_attr_cancel.attr,
>         &dev_attr_durations.attr,
>         &dev_attr_timeouts.attr,
> +       &dev_attr_version_major.attr,
>         NULL,
>  };
>  

The TPM version seems to be included in "dev_attr_caps.attr".

> -static const struct attribute_group tpm_dev_group = {
> -       .attrs = tpm_dev_attrs,
> +static struct attribute *tpm20_dev_attrs[] = {
> +       &dev_attr_version_major.attr,
> +       NULL
> +};

This should work, but wouldn't exporting this information under
security/tpmX, like the binary_bios_measurements, be a lot easier to
find and use?

Mimi

> +
> +static const struct attribute_group tpm12_dev_group = {
> +       .attrs = tpm12_dev_attrs,
> +};
> +
> +static const struct attribute_group tpm20_dev_group = {
> +       .attrs = tpm20_dev_attrs,
>  };
>  
>  void tpm_sysfs_add_device(struct tpm_chip *chip)
>  {
> -       if (chip->flags & TPM_CHIP_FLAG_TPM2)
> -               return;
> -
>         WARN_ON(chip->groups_cnt != 0);
> -       chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> +       if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +               chip->groups[chip->groups_cnt++] = &tpm20_dev_group;
> +       else
> +               chip->groups[chip->groups_cnt++] = &tpm12_dev_group;
>  }
> 
> 
> Did a quick test on 2 systems here.

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Jerry Snitselaar]

On Thu Oct 24 19, Mimi Zohar wrote:
>On Thu, 2019-10-24 at 14:38 -0700, Jerry Snitselaar wrote:
>> On Thu Oct 24 19, Jarkko Sakkinen wrote:
>> >On Thu, Oct 24, 2019 at 02:18:48PM +0200, Petr Vorel wrote:
>> >> Hi all,
>> >>
>> >> I wonder what to do with this patch "ima: skip verifying TPM 2.0 PCR values" [1].
>> >> Is it a correct way to differentiate between TPM 1.2 and TPM 2.0?
>> >> Or something else should be applied?
>> >>
>> >> How is the work on TPM 2.0 Linux sysfs interface?
>> >> But even it's done in near future, we'd still need some way for older kernels.
>> >>
>> >> Kind regards,
>> >> Petr
>> >>
>> >> [1] https://patchwork.ozlabs.org/patch/1100733/
>> >
>> >version_major sysfs file would be acceptable if someone wants to proceed
>> >and send such patch.
>> >
>> >Also replicants for durations and timeouts files would make sense for
>> >TPM 2.0.
>> >
>> >/Jarkko
>>
>> Is it as simple as doing this?
>>
>> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
>> index edfa89160010..fd8eb8d8945c 100644
>> --- a/drivers/char/tpm/tpm-sysfs.c
>> +++ b/drivers/char/tpm/tpm-sysfs.c
>> @@ -309,7 +309,17 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
>>  }
>>  static DEVICE_ATTR_RO(timeouts);
>>
>> -static struct attribute *tpm_dev_attrs[] = {
>> +static ssize_t version_major_show(struct device *dev,
>> +                                 struct device_attribute *attr, char *buf)
>> +{
>> +       struct tpm_chip *chip = to_tpm_chip(dev);
>> +
>> +       return sprintf(buf, "TPM%s\n", chip->flags & TPM_CHIP_FLAG_TPM2
>> +                      ? "2.0" : "1.2");
>> +}
>> +static DEVICE_ATTR_RO(version_major);
>> +
>> +static struct attribute *tpm12_dev_attrs[] = {
>>         &dev_attr_pubek.attr,
>>         &dev_attr_pcrs.attr,
>>         &dev_attr_enabled.attr,
>> @@ -320,18 +330,28 @@ static struct attribute *tpm_dev_attrs[] = {
>>         &dev_attr_cancel.attr,
>>         &dev_attr_durations.attr,
>>         &dev_attr_timeouts.attr,
>> +       &dev_attr_version_major.attr,
>>         NULL,
>>  };
>>
>
>The TPM version seems to be included in "dev_attr_caps.attr".
>
>> -static const struct attribute_group tpm_dev_group = {
>> -       .attrs = tpm_dev_attrs,
>> +static struct attribute *tpm20_dev_attrs[] = {
>> +       &dev_attr_version_major.attr,
>> +       NULL
>> +};
>
>This should work, but wouldn't exporting this information under
>security/tpmX, like the binary_bios_measurements, be a lot easier to
>find and use?
>
>Mimi
>

/sys/kernel/security/tpmX/major_version (on fedora and rhel at least, is it elsewhere on other distros?)

versus

/sys/class/tpm/tpmX/major_version

I don't know that it is any easier to find.

>> +
>> +static const struct attribute_group tpm12_dev_group = {
>> +       .attrs = tpm12_dev_attrs,
>> +};
>> +
>> +static const struct attribute_group tpm20_dev_group = {
>> +       .attrs = tpm20_dev_attrs,
>>  };
>>
>>  void tpm_sysfs_add_device(struct tpm_chip *chip)
>>  {
>> -       if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> -               return;
>> -
>>         WARN_ON(chip->groups_cnt != 0);
>> -       chip->groups[chip->groups_cnt++] = &tpm_dev_group;
>> +       if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> +               chip->groups[chip->groups_cnt++] = &tpm20_dev_group;
>> +       else
>> +               chip->groups[chip->groups_cnt++] = &tpm12_dev_group;
>>  }
>>
>>
>> Did a quick test on 2 systems here.
>

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Petr Vorel]

Hi,

> /sys/kernel/security/tpmX/major_version (on fedora and rhel at least, is it elsewhere on other distros?)

> versus

> /sys/class/tpm/tpmX/major_version

Is it more HW related (/sys/class/tpm/tpmX) or LSM related
(/sys/kernel/security/tpmX)?
I guess /sys/kernel/security/tpmX might be better.

Thanks for implementing this, I'll try to test it soon.

Kind regards,
Petr

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Serge E. Hallyn]

On Fri, Oct 25, 2019 at 10:56:17AM +0200, Petr Vorel wrote:
> Hi,
> 
> > /sys/kernel/security/tpmX/major_version (on fedora and rhel at least, is it elsewhere on other distros?)
> 
> > versus
> 
> > /sys/class/tpm/tpmX/major_version
> 
> Is it more HW related (/sys/class/tpm/tpmX) or LSM related
> (/sys/kernel/security/tpmX)?
> I guess /sys/kernel/security/tpmX might be better.

This is purely about whether the phsyical TPM chip is 1.2 or 2.,
right?  /sys/class/tpm/tpmX is where I would expect to find that.

> Thanks for implementing this, I'll try to test it soon.

Yes, it's been a pain point, and someone (..., I) should have done this years
ago - thanks!

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Mimi Zohar]

On Fri, 2019-10-25 at 07:52 -0500, Serge E. Hallyn wrote:
> On Fri, Oct 25, 2019 at 10:56:17AM +0200, Petr Vorel wrote:
> > Hi,
> > 
> > > /sys/kernel/security/tpmX/major_version (on fedora and rhel at
> least, is it elsewhere on other distros?)

This patch doesn't define a securityfs file.  It must be a soft link
to the actual file.

> > > versus
> > 
> > > /sys/class/tpm/tpmX/major_version

This is a softlink to the TPM device (eg.
/sys/devices/xxxx/.../tpm/tpm0).

> > 
> > Is it more HW related (/sys/class/tpm/tpmX) or LSM related
> > (/sys/kernel/security/tpmX)?
> > I guess /sys/kernel/security/tpmX might be better.
> 
> This is purely about whether the phsyical TPM chip is 1.2 or 2.,
> right?  /sys/class/tpm/tpmX is where I would expect to find that.
> 
> > Thanks for implementing this, I'll try to test it soon.
> 
> Yes, it's been a pain point, and someone (..., I) should have done this years
> ago - thanks!

+1

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Petr Vorel]

Hi,

> > > /sys/kernel/security/tpmX/major_version (on fedora and rhel at least, is it elsewhere on other distros?)

> > > versus

> > > /sys/class/tpm/tpmX/major_version

> > Is it more HW related (/sys/class/tpm/tpmX) or LSM related
> > (/sys/kernel/security/tpmX)?
> > I guess /sys/kernel/security/tpmX might be better.

> This is purely about whether the phsyical TPM chip is 1.2 or 2.,
> right?  /sys/class/tpm/tpmX is where I would expect to find that.
+1

> > Thanks for implementing this, I'll try to test it soon.

> Yes, it's been a pain point, and someone (..., I) should have done this years
> ago - thanks!

Kind regards,
Petr

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Jerry Snitselaar]

On Fri Oct 25 19, Petr Vorel wrote:
>Hi,
>
>> /sys/kernel/security/tpmX/major_version (on fedora and rhel at least, is it elsewhere on other distros?)
>
>> versus
>
>> /sys/class/tpm/tpmX/major_version
>
>Is it more HW related (/sys/class/tpm/tpmX) or LSM related
>(/sys/kernel/security/tpmX)?
>I guess /sys/kernel/security/tpmX might be better.
>

I think it is HW related since it is describing the
TCG version that the chip implements.

>Thanks for implementing this, I'll try to test it soon.
>
>Kind regards,
>Petr

Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values

[Jarkko Sakkinen]

On Thu, Oct 24, 2019 at 08:36:02PM -0300, Jason Gunthorpe wrote:
> On Thu, Oct 24, 2019 at 10:14:02PM +0300, Jarkko Sakkinen wrote:
> > On Thu, Oct 24, 2019 at 03:20:05PM -0300, Jason Gunthorpe wrote:
> > > On Thu, Oct 24, 2019 at 08:20:23PM +0300, Jarkko Sakkinen wrote:
> > > > Also replicants for durations and timeouts files would make sense for
> > > > TPM 2.0.
> > > 
> > > These ones don't meet the sysfs standard of one value per file, which
> > > is why they didn't make it to tpm2
> > 
> > They would be still useful to have available in some form as there is
> > no way deduce them from the user space.
> 
> Why? Userspace doesn't refer to these values since the kernel handles
> all the timeouts, right?

For debugging at least would be definitely a nice to have what
values the driver ended up setting.

/Jarkko