diff mbox series

[v11,18/20] tpm: Add sysfs interface to allow setting and querying the default locality

Message ID 20240913200517.3085794-19-ross.philipson@oracle.com (mailing list archive)
State New
Headers show
Series x86: Trenchboot secure dynamic launch Linux kernel support | expand

Commit Message

Ross Philipson Sept. 13, 2024, 8:05 p.m. UTC
Expose a sysfs interface to allow user mode to set and query the default
locality set for the TPM chip.

Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
---
 drivers/char/tpm/tpm-sysfs.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

Comments

James Bottomley Nov. 1, 2024, 3:17 a.m. UTC | #1
On Fri, 2024-09-13 at 13:05 -0700, Ross Philipson wrote:
> Expose a sysfs interface to allow user mode to set and query the
> default locality set for the TPM chip.

What does a user need this for?  It somewhat conflicts with the idea of
running the kernel and user space TPM access in separate localities for
the purposes of key release, so we can seal keys to only release in the
kernel by policy.  When I last talked about this I thought we'd
probably use 0 for user and, say 2, for the kernel (mainly because
prior incarnations of this patch set seemed to access the TPM in
locality 2 from the kernel).  It really doesn't matter *what* locality
we use for the kernel and the user as long as it's known ahead of time
and the user can't gain access to the kernel locality.

Regards,

James
Jarkko Sakkinen Nov. 1, 2024, 10:06 a.m. UTC | #2
On Fri Sep 13, 2024 at 11:05 PM EEST, Ross Philipson wrote:
> Expose a sysfs interface to allow user mode to set and query the default
> locality set for the TPM chip.
>
> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>

Must be read-only. Should be decided per power cycle.

BR, Jarkko
Jarkko Sakkinen Nov. 1, 2024, 9:50 p.m. UTC | #3
On Fri Nov 1, 2024 at 12:06 PM EET, Jarkko Sakkinen wrote:
> On Fri Sep 13, 2024 at 11:05 PM EEST, Ross Philipson wrote:
> > Expose a sysfs interface to allow user mode to set and query the default
> > locality set for the TPM chip.
> >
> > Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
>
> Must be read-only. Should be decided per power cycle.

I'm throwing one incomplete idea not all things considered...

So one idea is would be to apply set operation to /dev/tpm0 as ioctl
(would not be available for /dev/tpmrm0).

Then at least access control rules would apply.

The open here is that the IMA etc. will use a different locality during
boot-time, like it would also with sysfs attribute.

BR, Jarkko
Jarkko Sakkinen Nov. 1, 2024, 9:56 p.m. UTC | #4
On Fri Nov 1, 2024 at 11:50 PM EET, Jarkko Sakkinen wrote:
> On Fri Nov 1, 2024 at 12:06 PM EET, Jarkko Sakkinen wrote:
> > On Fri Sep 13, 2024 at 11:05 PM EEST, Ross Philipson wrote:
> > > Expose a sysfs interface to allow user mode to set and query the default
> > > locality set for the TPM chip.
> > >
> > > Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
> >
> > Must be read-only. Should be decided per power cycle.
>
> I'm throwing one incomplete idea not all things considered...
>
> So one idea is would be to apply set operation to /dev/tpm0 as ioctl
> (would not be available for /dev/tpmrm0).
>
> Then at least access control rules would apply.
>
> The open here is that the IMA etc. will use a different locality during
> boot-time, like it would also with sysfs attribute.

Looking at [1] this would become a problem if TPM2_PolicyLocality based
policy is ever used during boot-time.

We can make a choice of not allowing such policies for in-kernel clients
if agree so, but it is a choice that needs to be locked in. With quick
thinking I'm not sure if that is horrible limitation.

Also does not obviously affect clients communicating with /dev/tpm0.
With that constrain it would not matter if during boot-time different
locale is used.

With that constraint and "set" in ioctl instead of sysfs attributes
that might even work out... Open for feedback.

[1] https://trustedcomputinggroup.org/wp-content/uploads/TPM-2.0-1.83-Part-3-Commands.pdf

BR, Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 94231f052ea7..185a2f57d4cb 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -324,6 +324,34 @@  static ssize_t null_name_show(struct device *dev, struct device_attribute *attr,
 static DEVICE_ATTR_RO(null_name);
 #endif
 
+static ssize_t default_locality_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct tpm_chip *chip = to_tpm_chip(dev);
+
+	return sprintf(buf, "%d\n", chip->default_locality);
+}
+
+static ssize_t default_locality_store(struct device *dev, struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct tpm_chip *chip = to_tpm_chip(dev);
+	unsigned int locality;
+
+	if (kstrtouint(buf, 0, &locality))
+		return -ERANGE;
+
+	if (locality >= TPM_MAX_LOCALITY)
+		return -ERANGE;
+
+	if (tpm_chip_set_default_locality(chip, (int)locality))
+		return count;
+	else
+		return 0;
+}
+
+static DEVICE_ATTR_RW(default_locality);
+
 static struct attribute *tpm1_dev_attrs[] = {
 	&dev_attr_pubek.attr,
 	&dev_attr_pcrs.attr,
@@ -336,6 +364,7 @@  static struct attribute *tpm1_dev_attrs[] = {
 	&dev_attr_durations.attr,
 	&dev_attr_timeouts.attr,
 	&dev_attr_tpm_version_major.attr,
+	&dev_attr_default_locality.attr,
 	NULL,
 };
 
@@ -344,6 +373,7 @@  static struct attribute *tpm2_dev_attrs[] = {
 #ifdef CONFIG_TCG_TPM2_HMAC
 	&dev_attr_null_name.attr,
 #endif
+	&dev_attr_default_locality.attr,
 	NULL
 };