Linux-Integrity Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] ima: skip verifying TPM 2.0 PCR values
@ 2019-05-16 21:12 Mimi Zohar
  2019-05-17  6:51 ` Petr Vorel
  2019-05-17 13:50 ` Nayna
  0 siblings, 2 replies; 21+ messages in thread
From: Mimi Zohar @ 2019-05-16 21:12 UTC (permalink / raw)
  To: linux-integrity; +Cc: ltp, Petr Vorel

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-05-16 21:12 [PATCH] ima: skip verifying TPM 2.0 PCR values Mimi Zohar
@ 2019-05-17  6:51 ` Petr Vorel
  2019-05-17 11:19   ` Mimi Zohar
  2019-05-17 13:50 ` Nayna
  1 sibling, 1 reply; 21+ messages in thread
From: Petr Vorel @ 2019-05-17  6:51 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-integrity, ltp

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-05-17  6:51 ` Petr Vorel
@ 2019-05-17 11:19   ` Mimi Zohar
  2019-05-17 11:28     ` Petr Vorel
  0 siblings, 1 reply; 21+ messages in thread
From: Mimi Zohar @ 2019-05-17 11:19 UTC (permalink / raw)
  To: Petr Vorel
  Cc: linux-integrity, ltp, Jason Gunthorpe, Peter Hüwe, Jarkko Sakkinen

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"
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-05-17 11:19   ` Mimi Zohar
@ 2019-05-17 11:28     ` Petr Vorel
  0 siblings, 0 replies; 21+ messages in thread
From: Petr Vorel @ 2019-05-17 11:28 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-integrity, ltp, Jason Gunthorpe, Peter Hüwe, Jarkko Sakkinen

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-05-16 21:12 [PATCH] ima: skip verifying TPM 2.0 PCR values Mimi Zohar
  2019-05-17  6:51 ` Petr Vorel
@ 2019-05-17 13:50 ` Nayna
  2019-05-17 15:04   ` Petr Vorel
  1 sibling, 1 reply; 21+ messages in thread
From: Nayna @ 2019-05-17 13:50 UTC (permalink / raw)
  To: Mimi Zohar, linux-integrity; +Cc: ltp, Petr Vorel



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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-05-17 13:50 ` Nayna
@ 2019-05-17 15:04   ` Petr Vorel
  2019-10-24 12:18     ` [LTP] " Petr Vorel
  0 siblings, 1 reply; 21+ messages in thread
From: Petr Vorel @ 2019-05-17 15:04 UTC (permalink / raw)
  To: Nayna; +Cc: Mimi Zohar, linux-integrity, ltp

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-05-17 15:04   ` Petr Vorel
@ 2019-10-24 12:18     ` " Petr Vorel
  2019-10-24 17:20       ` Jarkko Sakkinen
  0 siblings, 1 reply; 21+ messages in thread
From: Petr Vorel @ 2019-10-24 12:18 UTC (permalink / raw)
  To: Nayna, Mimi Zohar, Jarkko Sakkinen
  Cc: linux-integrity, ltp, Piotr Król, Peter Huewe, Jason Gunthorpe

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/

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-24 12:18     ` [LTP] " Petr Vorel
@ 2019-10-24 17:20       ` Jarkko Sakkinen
  2019-10-24 18:20         ` Jason Gunthorpe
  2019-10-24 21:38         ` Jerry Snitselaar
  0 siblings, 2 replies; 21+ messages in thread
From: Jarkko Sakkinen @ 2019-10-24 17:20 UTC (permalink / raw)
  To: Petr Vorel
  Cc: Nayna, Mimi Zohar, linux-integrity, ltp, Piotr Król,
	Peter Huewe, Jason Gunthorpe

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-24 17:20       ` Jarkko Sakkinen
@ 2019-10-24 18:20         ` Jason Gunthorpe
  2019-10-24 19:14           ` Jarkko Sakkinen
  2019-10-24 21:38         ` Jerry Snitselaar
  1 sibling, 1 reply; 21+ messages in thread
From: Jason Gunthorpe @ 2019-10-24 18:20 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Petr Vorel, Nayna, Mimi Zohar, linux-integrity, ltp,
	Piotr Król, Peter Huewe

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-24 18:20         ` Jason Gunthorpe
@ 2019-10-24 19:14           ` Jarkko Sakkinen
  2019-10-24 23:36             ` Jason Gunthorpe
  0 siblings, 1 reply; 21+ messages in thread
From: Jarkko Sakkinen @ 2019-10-24 19:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Petr Vorel, Nayna, Mimi Zohar, linux-integrity, ltp,
	Piotr Król, Peter Huewe

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-24 17:20       ` Jarkko Sakkinen
  2019-10-24 18:20         ` Jason Gunthorpe
@ 2019-10-24 21:38         ` Jerry Snitselaar
  2019-10-24 23:26           ` Jason Gunthorpe
  2019-10-25  0:47           ` Mimi Zohar
  1 sibling, 2 replies; 21+ messages in thread
From: Jerry Snitselaar @ 2019-10-24 21:38 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Petr Vorel, Nayna, Mimi Zohar, linux-integrity, ltp,
	Piotr Król, Peter Huewe, Jason Gunthorpe

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.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-24 21:38         ` Jerry Snitselaar
@ 2019-10-24 23:26           ` Jason Gunthorpe
  2019-10-25  0:47           ` Mimi Zohar
  1 sibling, 0 replies; 21+ messages in thread
From: Jason Gunthorpe @ 2019-10-24 23:26 UTC (permalink / raw)
  To: Jerry Snitselaar
  Cc: Jarkko Sakkinen, Petr Vorel, Nayna, Mimi Zohar, linux-integrity,
	ltp, Piotr Król, Peter Huewe

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-24 19:14           ` Jarkko Sakkinen
@ 2019-10-24 23:36             ` Jason Gunthorpe
  2019-10-28 20:51               ` Jarkko Sakkinen
  0 siblings, 1 reply; 21+ messages in thread
From: Jason Gunthorpe @ 2019-10-24 23:36 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Petr Vorel, Nayna, Mimi Zohar, linux-integrity, ltp,
	Piotr Król, Peter Huewe

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-24 21:38         ` Jerry Snitselaar
  2019-10-24 23:26           ` Jason Gunthorpe
@ 2019-10-25  0:47           ` Mimi Zohar
  2019-10-25  2:11             ` Jerry Snitselaar
  1 sibling, 1 reply; 21+ messages in thread
From: Mimi Zohar @ 2019-10-25  0:47 UTC (permalink / raw)
  To: Jerry Snitselaar, Jarkko Sakkinen
  Cc: Petr Vorel, Nayna, linux-integrity, ltp, Piotr Król,
	Peter Huewe, Jason Gunthorpe

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.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-25  0:47           ` Mimi Zohar
@ 2019-10-25  2:11             ` Jerry Snitselaar
  2019-10-25  8:56               ` Petr Vorel
  0 siblings, 1 reply; 21+ messages in thread
From: Jerry Snitselaar @ 2019-10-25  2:11 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Jarkko Sakkinen, Petr Vorel, Nayna, linux-integrity, ltp,
	Piotr Król, Peter Huewe, Jason Gunthorpe

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.
>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-25  2:11             ` Jerry Snitselaar
@ 2019-10-25  8:56               ` Petr Vorel
  2019-10-25 12:52                 ` Serge E. Hallyn
  2019-10-25 14:13                 ` Jerry Snitselaar
  0 siblings, 2 replies; 21+ messages in thread
From: Petr Vorel @ 2019-10-25  8:56 UTC (permalink / raw)
  To: Jerry Snitselaar
  Cc: Mimi Zohar, Jarkko Sakkinen, Nayna, linux-integrity, ltp,
	Piotr Król, Peter Huewe, Jason Gunthorpe

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-25  8:56               ` Petr Vorel
@ 2019-10-25 12:52                 ` Serge E. Hallyn
  2019-10-25 13:22                   ` Mimi Zohar
  2019-10-25 13:25                   ` Petr Vorel
  2019-10-25 14:13                 ` Jerry Snitselaar
  1 sibling, 2 replies; 21+ messages in thread
From: Serge E. Hallyn @ 2019-10-25 12:52 UTC (permalink / raw)
  To: Petr Vorel
  Cc: Jerry Snitselaar, Mimi Zohar, Jarkko Sakkinen, Nayna,
	linux-integrity, ltp, Piotr Król, Peter Huewe,
	Jason Gunthorpe

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!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-25 12:52                 ` Serge E. Hallyn
@ 2019-10-25 13:22                   ` Mimi Zohar
  2019-10-25 13:25                   ` Petr Vorel
  1 sibling, 0 replies; 21+ messages in thread
From: Mimi Zohar @ 2019-10-25 13:22 UTC (permalink / raw)
  To: Serge E. Hallyn, Petr Vorel
  Cc: Jerry Snitselaar, Jarkko Sakkinen, Nayna, linux-integrity, ltp,
	Piotr Król, Peter Huewe, Jason Gunthorpe

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-25 12:52                 ` Serge E. Hallyn
  2019-10-25 13:22                   ` Mimi Zohar
@ 2019-10-25 13:25                   ` Petr Vorel
  1 sibling, 0 replies; 21+ messages in thread
From: Petr Vorel @ 2019-10-25 13:25 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: Jerry Snitselaar, Mimi Zohar, Jarkko Sakkinen, Nayna,
	linux-integrity, ltp, Piotr Król, Peter Huewe,
	Jason Gunthorpe

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-25  8:56               ` Petr Vorel
  2019-10-25 12:52                 ` Serge E. Hallyn
@ 2019-10-25 14:13                 ` Jerry Snitselaar
  1 sibling, 0 replies; 21+ messages in thread
From: Jerry Snitselaar @ 2019-10-25 14:13 UTC (permalink / raw)
  To: Petr Vorel
  Cc: Mimi Zohar, Jarkko Sakkinen, Nayna, linux-integrity, ltp,
	Piotr Król, Peter Huewe, Jason Gunthorpe

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [LTP] [PATCH] ima: skip verifying TPM 2.0 PCR values
  2019-10-24 23:36             ` Jason Gunthorpe
@ 2019-10-28 20:51               ` Jarkko Sakkinen
  0 siblings, 0 replies; 21+ messages in thread
From: Jarkko Sakkinen @ 2019-10-28 20:51 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Petr Vorel, Nayna, Mimi Zohar, linux-integrity, ltp,
	Piotr Król, Peter Huewe

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, back to index

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-16 21:12 [PATCH] ima: skip verifying TPM 2.0 PCR values Mimi Zohar
2019-05-17  6:51 ` Petr Vorel
2019-05-17 11:19   ` Mimi Zohar
2019-05-17 11:28     ` Petr Vorel
2019-05-17 13:50 ` Nayna
2019-05-17 15:04   ` Petr Vorel
2019-10-24 12:18     ` [LTP] " Petr Vorel
2019-10-24 17:20       ` Jarkko Sakkinen
2019-10-24 18:20         ` Jason Gunthorpe
2019-10-24 19:14           ` Jarkko Sakkinen
2019-10-24 23:36             ` Jason Gunthorpe
2019-10-28 20:51               ` Jarkko Sakkinen
2019-10-24 21:38         ` Jerry Snitselaar
2019-10-24 23:26           ` Jason Gunthorpe
2019-10-25  0:47           ` Mimi Zohar
2019-10-25  2:11             ` Jerry Snitselaar
2019-10-25  8:56               ` Petr Vorel
2019-10-25 12:52                 ` Serge E. Hallyn
2019-10-25 13:22                   ` Mimi Zohar
2019-10-25 13:25                   ` Petr Vorel
2019-10-25 14:13                 ` Jerry Snitselaar

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-integrity linux-integrity/ https://lore.kernel.org/linux-integrity \
		linux-integrity@vger.kernel.org
	public-inbox-index linux-integrity

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-integrity


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git