ima: skip verifying TPM 2.0 PCR values

PATCH In review Mimi Zohar 7 participants 21 messages May 16, 2019 – Oct 28, 2019

Subsystems (2)

Keys-trusted TPM device driver

lore

Share & tools

lore

Mimi Zohar

author maintainer

2019-05-16 21:12 UTC #0 lore raw

PATCH 1/1 ima: skip verifying TPM 2.0 PCR values

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 <[email protected]>

---
 testcases/kernel/security/integrity/ima/tests/ima_tpm.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +8 -0

index 0ffc3c02247d..ebe4b4c360e4 100755
@@ -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"

Signature · 1 line

2.7.5

Petr Vorel

Acked-by on v1 1/1

2019-05-17 06:51 UTC #1 lore raw

Hi Mimi,

Quoted 4 lines

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 <[email protected]>

Acked-by Petr Vorel <[email protected]>

Quoted 9 lines

---
 testcases/kernel/security/integrity/ima/tests/ima_tpm.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +0 -0

index 0ffc3c02247d..ebe4b4c360e4 100755

testcases/kernel/security/integrity/ima/tests/ima_tpm.sh test2() · line 93

Quoted 6 lines

testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +3 -0

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

testcases/kernel/security/integrity/ima/tests/ima_tpm.sh test2() · line 101

Quoted 8 lines

testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +5 -0

+			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

Mimi Zohar

author maintainer

2019-05-17 11:19 UTC #2 lore raw

Quoted 8 lines · Petr Vorel

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

Quoted 8 lines

testcases/kernel/security/integrity/ima/tests/ima_tpm.sh test2() · line 93
Quoted 3 lines
testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +3 -0
+	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

Quoted 9 lines

Quoted 8 lines

testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +5 -0

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

Petr Vorel

2019-05-17 11:28 UTC #3 lore raw

Hi Mimi,

Quoted 19 lines · Petr Vorel

Quoted 9 lines
Quoted 8 lines
testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +0 -0
index 0ffc3c02247d..ebe4b4c360e4 100755
@@ -88,6 +88,14 @@ test2()
 	tst_res TINFO "verify PCR values"
 	tst_check_cmds evmctl
Quoted 8 lines
testcases/kernel/security/integrity/ima/tests/ima_tpm.sh test2() · line 93
Quoted 3 lines
testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +3 -0
+	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.

Quoted 2 lines

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.

Quoted 1 line

Mimi

Kind regards,
Petr

Nayna

2019-05-17 13:50 UTC #4 lore raw

In reply to Mimi Zohar

Quoted 13 lines · 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 <[email protected]>
---
  testcases/kernel/security/integrity/ima/tests/ima_tpm.sh | 8 ++++++++
  1 file changed, 8 insertions(+)
testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +0 -0
index 0ffc3c02247d..ebe4b4c360e4 100755

testcases/kernel/security/integrity/ima/tests/ima_tpm.sh test2() · line 91

Quoted 4 lines

testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +1 -0

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

Petr Vorel

2019-05-17 15:04 UTC #5 lore raw

Hi Nayna,

...

Quoted 1 line

Quoted 1 line
testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +1 -0
+	local tpm_description="/sys/class/tpm/tpm0/device/description"

...

Quoted 5 lines

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.

Quoted 2 lines

Thanks & Regards,
- Nayna

Kind regards,
Petr

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

Petr Vorel

2019-10-24 12:18 UTC #6 lore raw

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/

Jarkko Sakkinen

2019-10-24 17:20 UTC #7 lore raw

Quoted 6 lines · 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?

Quoted 7 lines

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

Jason Gunthorpe

reviewer

2019-10-24 18:20 UTC #8 lore raw

Quoted 2 lines · Jarkko Sakkinen

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

Jarkko Sakkinen

2019-10-24 19:14 UTC #9 lore raw

Quoted 6 lines · Jason Gunthorpe

Quoted 2 lines · Jarkko Sakkinen
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

Jason Gunthorpe

reviewer

2019-10-24 23:36 UTC #10 lore raw

Quoted 10 lines · Jarkko Sakkinen

Quoted 6 lines · Jason Gunthorpe
Quoted 2 lines · Jarkko Sakkinen
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

Jarkko Sakkinen

2019-10-28 20:51 UTC #11 lore raw

Quoted 6 lines · Jason Gunthorpe

Quoted 5 lines · Jarkko Sakkinen
Quoted 4 lines · Jason Gunthorpe
Quoted 2 lines · Jarkko Sakkinen
Also replicants for durations and timeouts files would make sense for TPM 2.0.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
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

Jerry Snitselaar

2019-10-24 21:38 UTC #12 lore raw

In reply to Jarkko Sakkinen

Quoted 15 lines · Jarkko Sakkinen

Quoted 6 lines · 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?
Quoted 7 lines
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/

Quoted 7 lines

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?

drivers/char/tpm/tpm-sysfs.c +27 -7

index edfa89160010..fd8eb8d8945c 100644
@@ -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.

Jason Gunthorpe

reviewer

2019-10-24 23:26 UTC #13 lore raw

Quoted 31 lines · Jerry Snitselaar

Quoted 15 lines · Jarkko Sakkinen
Quoted 6 lines · 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?
Quoted 7 lines
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/
Quoted 7 lines
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?
drivers/char/tpm/tpm-sysfs.c +1 -0
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,
 }

drivers/char/tpm/tpm-sysfs.c static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr, · line 318

Quoted 10 lines

drivers/char/tpm/tpm-sysfs.c +6 -1

 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

Mimi Zohar

author maintainer

2019-10-25 00:47 UTC #14 lore raw

In reply to Jerry Snitselaar

Quoted 50 lines · Jerry Snitselaar

Quoted 15 lines · Jarkko Sakkinen
Quoted 6 lines · 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?
Quoted 7 lines
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/
Quoted 7 lines
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?
drivers/char/tpm/tpm-sysfs.c +11 -1
index edfa89160010..fd8eb8d8945c 100644
@@ -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,

drivers/char/tpm/tpm-sysfs.c static struct attribute *tpm_dev_attrs[] = { · line 336

Quoted 7 lines

drivers/char/tpm/tpm-sysfs.c +1 -0

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

drivers/char/tpm/tpm-sysfs.c static struct attribute *tpm_dev_attrs[] = { · line 340

Quoted 6 lines

drivers/char/tpm/tpm-sysfs.c +4 -2

-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

Quoted 24 lines

drivers/char/tpm/tpm-sysfs.c +11 -4

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

Jerry Snitselaar

2019-10-25 02:11 UTC #15 lore raw

Quoted 72 lines · Mimi Zohar

Quoted 50 lines · Jerry Snitselaar
Quoted 15 lines · Jarkko Sakkinen
Quoted 6 lines · 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?
Quoted 7 lines
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/
Quoted 7 lines
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?
drivers/char/tpm/tpm-sysfs.c +11 -1
index edfa89160010..fd8eb8d8945c 100644
@@ -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,
drivers/char/tpm/tpm-sysfs.c static struct attribute *tpm_dev_attrs[] = { · line 336
Quoted 7 lines
drivers/char/tpm/tpm-sysfs.c +1 -0
@@ -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".
drivers/char/tpm/tpm-sysfs.c static struct attribute *tpm_dev_attrs[] = { · line 340
Quoted 6 lines
drivers/char/tpm/tpm-sysfs.c +4 -2
-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?

Quoted 2 lines

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.

Quoted 25 lines

Quoted 24 lines

drivers/char/tpm/tpm-sysfs.c +11 -4

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

Petr Vorel

2019-10-25 08:56 UTC #16 lore raw

Hi,

Quoted 5 lines

/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

Serge E. Hallyn

2019-10-25 12:52 UTC #17 lore raw

Quoted 11 lines · Petr Vorel

Hi,
Quoted 5 lines
/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.

Quoted 1 line

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!

Mimi Zohar

author maintainer

2019-10-25 13:22 UTC #18 lore raw

Quoted 5 lines · Serge E. Hallyn

Quoted 3 lines · Petr Vorel
Hi,
Quoted 1 line
/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.

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
versus
/sys/class/tpm/tpmX/major_version

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

Quoted 5 lines

Quoted 4 lines
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.

Quoted 7 lines

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.
Quoted 1 line
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!

Petr Vorel

2019-10-25 13:25 UTC #19 lore raw

In reply to Serge E. Hallyn

Hi,

Quoted 4 lines

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

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
/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.

Quoted 4 lines

Quoted 1 line
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

Jerry Snitselaar

2019-10-25 14:13 UTC #20 lore raw

In reply to Petr Vorel

Quoted 4 lines · Petr Vorel

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

Quoted 8 lines

Quoted 3 lines
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.

Quoted 4 lines

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